Skip to content
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

pipeline: generic interface for pipeline items #378

Merged
merged 6 commits into from Nov 27, 2020

Conversation

ntessore
Copy link
Member

Provide a generic base class Item for pipeline items, and two concretisations Ref (reference to another variable) and Call (function call). The pipeline items have three methods:

  • infer to infer additional properties from context (e.g. the cosmology argument in Call),
  • depend to return the dependencies of the item, and
  • evaluate to return the computed value of the item.

This removes special cases from the Pipeline class and makes it easily extendable to other types of items.

@ntessore ntessore requested a review from a team as a code owner November 21, 2020 10:08
@@ -34,6 +32,12 @@ def test_load_skypy_yaml():
load_skypy_yaml(filename)


def test_empty_ref():
filename = get_pkg_data_filename('data/test_empty_ref.yml')
with pytest.raises(ValueError, match='empty reference'):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the yml file for this test, is this still an empty reference? Or a broken one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking here was that at the config (i.e. YAML) level we can only catch empty strings being passed when the !ref tag is used without argument. We could also let it go to the pipeline level and check there, but then we lose the information about where the error happened.

Which now makes me think that the next iteration of this should be adding 'file', 'line' and 'column' information to Item on construction so that the pipeline can do all of the checking and give great error messages.

skypy/pipeline/_pipeline.py Show resolved Hide resolved
skypy/pipeline/tests/test_items.py Show resolved Hide resolved
skypy/pipeline/tests/test_items.py Show resolved Hide resolved
@rrjbca rrjbca merged commit 3ab3842 into skypyproject:master Nov 27, 2020
@ntessore ntessore deleted the pipeline-items branch November 27, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants