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

tickets/DM-14216: Remove filter from metadata search #76

Merged
merged 2 commits into from May 4, 2018

Conversation

SimonKrughoff
Copy link
Contributor

For specs that don't depend on filter, the filter_name can simply be omitted.

For specs that don't depend on filter, the filter_name can simply be omitted.
@jonathansick
Copy link
Contributor

jonathansick commented May 2, 2018

So this is my best solution:

specs = release_specs.subset(
    required_metadata={'instrument':row['Instrument']})

specs = {name: spec for name, spec in specs.items()
         if 'filter_name' not in spec._metadata_query.terms
         or spec._metadata_query.terms['filter_name'] == row['Filer']}

Unfortunately, it relies on a private attribute, so this is a use case we'll want to better support in lsst.verify.

I think it would be good to expose those metadata query terms from the specification more readily.

The other thing I wonder is whether you can tag the specifications you want, and then query for those tags?

Copy link
Contributor

@jonathansick jonathansick left a comment

Choose a reason for hiding this comment

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

Looks good overall.

@@ -139,7 +144,7 @@ def run(repo_or_json, metrics=None,
return

json_path = repo_or_json
job = load_json_output(json_path)
job = load_json_output(json_path, **kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

What do we expect these kwargs to be? might be worth documenting a bit.

@SimonKrughoff SimonKrughoff merged commit 52a964b into master May 4, 2018
@ktlim ktlim deleted the tickets/DM-14216 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants