DM-38915: Fix handling of empty collection lists #835
Conversation
Empty collection iterable is now handled as it was supposed to be handled - producing empty result (with "doomed by" messages). I added a proper type annotation for `collections` parameter in Registry methods to help me verify that it is used correctly (maybe need to extend that to Butler methods too). Also needed to update couple of CLI scripts to fix their handling of the default collection values. Unit tests was added to check that it works correctly now.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #835 +/- ##
==========================================
+ Coverage 87.75% 87.76% +0.01%
==========================================
Files 268 268
Lines 35181 35246 +65
Branches 7407 7420 +13
==========================================
+ Hits 30873 30935 +62
Misses 3154 3154
- Partials 1154 1157 +3
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Looks good.
I don't think it's necessary to modify the butler methods right now unless you feel like it. It does seem like we should try to get to that point eventually. I had originally thought just using Any was fine here since we always conformed it with the wildcard class, but if it less this inconsistency slip through it's probably better to have this type alias.
| """Return true if both ``strings`` and ``patterns`` are empty.""" | ||
| # bool(Ellipsis) is True | ||
| return not self.strings and not self.patterns | ||
|
|
There was a problem hiding this comment.
Would it make sense to implement __bool__ instead, with the opposite sign?
There was a problem hiding this comment.
I thought about __bool__, but I decided that explicit method is better, it's too easy to mix check for True/False with check for None.
Empty collection iterable is now handled as it was supposed to be handled -
producing empty result (with "doomed by" messages). I added a proper type
annotation for
collectionsparameter in Registry methods to help meverify that it is used correctly (maybe need to extend that to Butler
methods too). Also needed to update couple of CLI scripts to fix their
handling of the default collection values. Unit tests was added to check
that it works correctly now.
Checklist
doc/changes