DM-25760: add --glob to butler query-collections #363
Conversation
n8pease
force-pushed
the
tickets/DM-25760
branch
from
544148e
to
dbaa49e
Compare
|
I see the current linter errors. I have to run out for about an hour and will address these when I get back. |
n8pease
force-pushed
the
tickets/DM-25760
branch
from
dbaa49e
to
39aa723
Compare
|
(all checks are passing now) |
python/lsst/daf/butler/core/utils.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| expressions : [`str` or `...`] |
python/lsst/daf/butler/core/utils.py
Outdated
| Parameters | ||
| ---------- | ||
| expressions : `list` [`str` or `...`] | ||
| A glob-style pattern string to convert, or an Ellipsis. |
There was a problem hiding this comment.
This docstring is now wrong (I saw the code originally had ellipsis and I had a long comment on not understanding that).
| @@ -47,7 +51,11 @@ def queryCollections(repo, collection_type, flatten_chains, include_chains): | |||
| collection names. | |||
| """ | |||
| butler = Butler(repo) | |||
| kwargs = {} | |||
| if glob: | |||
| kwargs['expression'] = globToRegex(glob) | |||
There was a problem hiding this comment.
From your earlier version handling of ... for this did make me wonder whether there is an efficiency gain for globToRegex to spot * as a specil case -- it probably should warn if * is passed in with other globs and if they just pass in * it should probably return an empty list rather than a regex if we assume that empty list means no constraints (that would save a lot of unnecessary regex processing when we know there is no filtering).
| @@ -47,7 +51,11 @@ def queryCollections(repo, collection_type, flatten_chains, include_chains): | |||
| collection names. | |||
| """ | |||
| butler = Butler(repo) | |||
| kwargs = {} | |||
| if glob: | |||
There was a problem hiding this comment.
since globToRegex can take an empty list it might be clearer to write this as:
filter = globToRegex(glob)
if filter:
kwargs["expression"] = filter| collections = butler.registry.queryCollections(collectionType=collection_type, | ||
| flattenChains=flatten_chains, | ||
| includeChains=include_chains) | ||
| includeChains=include_chains, | ||
| **kwargs) | ||
| return {'collections': list(collections)} |
There was a problem hiding this comment.
Single quotes have crept into this file.
tests/test_utils.py
Outdated
| globToRegex function api. | ||
| """ | ||
| testval = ["foo*", "bar"] | ||
| self.assertEqual(globToRegex(testval), |
There was a problem hiding this comment.
I think a better test is to check that the result of globToRegex really does work properly. ie does it return something that matches bar and not bars or abar and matches food and not flood.
use kwargs var for conditionally passed var add type string for glob argument
n8pease
force-pushed
the
tickets/DM-25760
branch
from
39aa723
to
58fbb94
Compare
No description provided.