Details
-
Type:
Story
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: daf_butler, pipe_base
-
Labels:
-
Story Points:0.5
-
Team:Data Release Production
-
Urgent?:No
Description
In the flurry of late-night merging for DM-26629, I forgot to do (at least) two things:
- I added one post-review feature commit to daf_butler to enable simple calibration lookups in butler.get. This was used (and hence tested) in ci_cpp_gen3, but it merits tests in daf_butler and probably some additional documentation, as well as a review.
- I promised in the review to update some documentation in pipe_base about ButlerQuantumContext preconditions, and then forgot to actually do so.
Happily none of them involve schema changes (or even non-test code), so it's probably good that they just got deferred to another ticket instead of holding up DM-26629.
Attachments
Issue Links
- relates to
-
DM-26953 Update ap_verify system for calibration collections
- Done
Activity
I have been wondering about the testing situation. test_butler.py has blown up quite spectacularly and could do with refactoring. The more I shrink the special cases of datastore (I can see how to get it down to two classes for file I/O but possibly even 1) the less we really need to try out every single datastore combination in test_butler.py – I think it would be a big win to factor out ButlerURI into a separate package that could go on pypi since it's really generic and that would lower the bar ever further for external users adding new remote datastores. When I added my registry test to test_butler.py I had been wondering why we don't have a distinct set of butler tests that mainly worry about registry and don't really care what datastore does with it. You can envisage butler tests where you can switch registry backend but always use simplest possible in-memory datastore (so no need to get bogged down with formatters and file templates). All this rambling is to say refactoring is good and test_butler could do with it as well.
Took my name off reviewer list
Tim Jenness, I've added an obs_base branch for some breakage that Krzysztof Findeisen found earlier today. Mind taking a look at that, too? After that I'm going to call this ticket done, though, and open new tickets if we find more problems. Jenkins is running.
Glad to hear this is at least not inconsistent with your thoughts about refactoring test_butler.py. I have no idea how many of our existing tests could work with a mock Datastore instead of an in-memory one, but I figure we've now at least got a place to put any such tests.
Tim Jenness, could you look at daf_butler? I'm hoping the revamped test file might prove useful for you down the line as well, and I'm open to suggestions on how to make that more true.
Nate Lust, could you look at pipe_base? This is just doc-only attempt to address your concerns about the new resolved-DatasetRefs-only precondition on ButlerQuantumContext - basically just documenting the precondition and acknowledging that it's a problem that the easiest way to satisfy the precondition is in a downstream package.