Details
-
Type:
Bug
-
Status: To Do
-
Resolution: Unresolved
-
Fix Version/s: None
-
Component/s: daf_butler
-
Labels:None
-
Team:Data Release Production
-
Urgent?:No
Description
Calling lsst.daf.butler.Registry.queryCollections with the name of a collection that is not present raises MissingCollectionError. From the documentation, I expect it to return an iterator over an empty iterable instead.
Minimal example:
from lsst.daf.butler import Butler
|
butler = Butler(Butler.makeRepo("foo"))
|
list(butler.registry.queryCollections("collected"))
|
The same behavior occurs when passing ["collected"]. Passing re.compile("collected") returns an empty iterator, as expected. I have not tested a tuple or a mapping, since I'm not sure how to construct a valid example.
Attachments
Issue Links
- relates to
-
DM-21915 Support Gen 3 ingestion of ap_verify datasets
- Done
Activity
I think consistency between multiple calls to the same function is much more important. If a function does one thing when called with A and a completely different thing when called with B, it should be two functions.
For me, at least, "query" implies non-raising behavior (except in cases where e.g. the query cannot even be executed). That's a different situation from an operation that assumes a target collection, which is what I assume you mean by "most similar interfaces".
If you want to keep the same implementation, and the low-level code raises, why not simply catch it inside queryCollections?
Krzysztof Findeisen
added a comment - - edited I think consistency between multiple calls to the same function is much more important. If a function does one thing when called with A and a completely different thing when called with B, it should be two functions.
For me, at least, "query" implies non-raising behavior (except in cases where e.g. the query cannot even be executed). That's a different situation from an operation that assumes a target collection, which is what I assume you mean by "most similar interfaces".
If you want to keep the same implementation, and the low-level code raises, why not simply catch it inside queryCollections ? Agree with all of the above. I'll certainly try catching the exception first; it might be trickier than that because the interface we're calling probably returns an iterable, and we don't want an exception to short-circuit that, but I'm sure there's some level we can catch it, even if we have to pass a raise/no-raise flag argument through a few levels to get there.
Seeing the exact call makes it clear what's going on, and I suppose my answer about whether this is expected behavior would change. But it's still worth discussing whether it's desirable behavior - I agree that an empty iterator for this input is more intuitive. But in most similar interfaces where collection expressions can be passed (generally in searches for datasets across multiple collections), I think getting this exception is clearly more desirable than an empty iterator of datasets (and that's why the code does raise; it's the same logic under the hood). Is consistency between those interfaces enough to make it a good idea to keep the raise?