-
Notifications
You must be signed in to change notification settings - Fork 2
Dynamic checking of requirement steps in pipelines #122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
pgierz
wants to merge
8
commits into
main
Choose a base branch
from
feat/cell_methods
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9ce79ba
wip: dynamic requirement checking of steps in a pipeline
pgierz d854642
wip: typo
pgierz 401c80d
fix: mini typo in new from_dict for rule crosschecks
pgierz bdb5eca
wip
pgierz 3d0ff6f
wip: adds standalone checker
pgierz c7d1e1b
feat: configurable crosschecking
pgierz 9a19802
correct example yaml
pgierz ef95b92
Merge branch 'main' into feat/cell_methods
pgierz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import copy | ||
| import datetime | ||
| import inspect | ||
| import pathlib | ||
| import re | ||
| import typing | ||
|
|
@@ -12,6 +13,54 @@ | |
| from .data_request.variable import DataRequestVariable | ||
| from .gather_inputs import InputFileCollection | ||
| from .logging import logger | ||
| from .pipeline import Pipeline | ||
|
|
||
|
|
||
| class RuleRequirement: | ||
| """Defines a requirement which steps must have tagged in order to be valid""" | ||
|
|
||
| def __init__(self, requirement_name: str, requirement_value: typing.Any): | ||
| """ | ||
| Initialize a RuleRequirement object. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| requirement_name : str | ||
| The name of the requirement that the step must satisfy. | ||
| requirement_value : Any | ||
| The value of the requirement that the step must satisfy. | ||
| """ | ||
| self.requirement_name = requirement_name | ||
| self.requirement_value = requirement_value | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, d): | ||
| """Build a RuleRequirement object from a dictionary""" | ||
| return cls(**d) | ||
|
|
||
| def pipeline_fulfills_requirements(self, pipeline: Pipeline) -> bool: | ||
| """Check if a pipeline fulfills the requirements of the rule | ||
|
|
||
| Parameters | ||
| ---------- | ||
| pipeline : Pipeline | ||
| The pipeline to check | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| True if the pipeline fulfills the requirements, False otherwise | ||
| """ | ||
| for step in pipeline.steps: | ||
| step_src = inspect.getsource(step) | ||
| for line in step_src.split("\n"): | ||
| if f"_satisfies_{self.requirement_name}" in line: | ||
| if str(self.requirement_value) in line: | ||
| return True | ||
| return False | ||
|
|
||
| def __repr__(self): | ||
| return f"{self.__class__.__name__}({self.requirement_name}, {self.requirement_value})" | ||
|
|
||
|
|
||
| class Rule: | ||
|
|
@@ -167,6 +216,31 @@ def match_pipelines(self, pipelines, force=False): | |
| self.pipelines = matched_pipelines | ||
| self._pipelines_are_mapped = True | ||
|
|
||
| def crosscheck_pipelines(self): | ||
| """ | ||
| Check that all pipelines are valid for the rule. | ||
|
|
||
| This method will raise a ValueError if any of the pipelines in the rule are not valid. | ||
| """ | ||
| requirements = [] | ||
| # FIXME: Dynamically get requirements based on CMOR variable. Something like this: | ||
| # Should be a list of dictionaries containing requirement specifications: | ||
| # requirements = [{"requirement_name": "cell_methods", "requirement_value": "time: mean"}, ] | ||
| if hasattr(self.data_request_variable, "cell_methods"): | ||
| requirements.append( | ||
| { | ||
| "requirement_name": "cell_methods", | ||
| "requirement_value": self.data_request_variable.cell_methods, | ||
| }, | ||
| ) | ||
| for requirement in requirements: | ||
| rr = RuleRequirement.from_dict(requirement) | ||
| req_satisfied = any( | ||
| rr.pipeline_fulfills_requirements(pl) for pl in self.pipelines | ||
| ) | ||
| if not req_satisfied: | ||
| raise ValueError(f"Rule {self.name} does not satisfy requirement {rr}") | ||
|
Comment on lines
+241
to
+242
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this should gather all requirements, instead of exiting at the first one. |
||
|
|
||
| @classmethod | ||
| def from_dict(cls, data): | ||
| """Build a rule object from a dictionary | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not terribly convinced by this, it requires that the name and value are strictly defined to be on the same line. That might not be the best way of doing it.