DM-53428: add tooling for updating butler dimension regions from VisitGeometry datasets #538
Conversation
TallJimbo
force-pushed
the
tickets/DM-53428
branch
from
d81bb9d to
0879059
Compare
isullivan
left a comment
isullivan
left a comment
There was a problem hiding this comment.
A couple comments on checking consistency, which are really stretch goals for this ticket.
| visit_id = new_visit_record.id | ||
| old_visit_record = old_visit_records[visit_id] | ||
| visit_geometry = visit_geometries[visit_id] | ||
| self.assertNotEqual(new_visit_record.region, old_visit_record.region) |
There was a problem hiding this comment.
It might be too much of a pain, but could you also test that the new_visit_record.region equals the expected transformation of old_visit_record.region, since you know the rotation that was applied?
There was a problem hiding this comment.
That's effectively done in the line below, since that visit_geometry has the updated region.
| butler.registry.insertDimensionData("exposure", *new_exposure_records, replace=True) | ||
| butler.registry.insertDimensionData("visit", *new_visit_records, replace=True) | ||
| butler.registry.insertDimensionData("visit_detector_region", *new_vdr_records, replace=True) | ||
| _LOG.info("Records for %d visits updated.", len(all_refs)) |
There was a problem hiding this comment.
Would it be appropriate to add a warning message if the final number of updated records did not match the initial number expected?
There was a problem hiding this comment.
I don't think we have a way to get that information back from the database inside the butler insert calls. Note that the variable I'm taking the len of is the same in both that initial log message and this final one - I think the count is worth repeating because the second is the only level=INFO one.
TallJimbo
force-pushed
the
tickets/DM-53428
branch
from
0879059 to
3e049c4
Compare
Checklist
doc/changes