# Custom classes for DataUnit tuples/sets and Data IDs

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
10
• Team:
Data Release Production

## Description

In the review for DM-14334 it looked like some utility classes for DataUnits could be helpful.  In particular:

• We once had a DataUnitTuple class that defined a sort order and a de-duped set of value column names, but dropped it when we added the new DataUnit class.  Looks like we should create a new one that builds on the DataUnit class and TopologicalSet.
• We should at least consider having a custom Data ID class, both to make it hashable and hence usable as a dict key and as a place to add integer packing in the future.

## Attachments

 1 New design for DataUnit/Dimension objects Done Jim Bosch 2 Integrate new Dimension classes with Registry and Preflight Done Jim Bosch

## Activity

Hide
Jim Bosch added a comment -

Note: DM-15214 adds some custom ordering code for DataUnits that should be obsoleted and replaced by this ticket.

Show
Jim Bosch added a comment - Note: DM-15214 adds some custom ordering code for DataUnits that should be obsoleted and replaced by this ticket.
Hide
Jim Bosch added a comment -

Andy Salnikov, just a heads up that I'm starting on this now, and it'll involve a pretty thorough rewrite of the APIs for workings with DataUnits and DataUnitJoins, and hence it'll involve some big changes in the preflight code as well.

I'll ping you again once I start working on the changes to preflight, as we probably want to avoid us both making changes there at the same time (but I'm not there yet).

Show
Jim Bosch added a comment - Andy Salnikov , just a heads up that I'm starting on this now, and it'll involve a pretty thorough rewrite of the APIs for workings with DataUnits and DataUnitJoins, and hence it'll involve some big changes in the preflight code as well. I'll ping you again once I start working on the changes to preflight, as we probably want to avoid us both making changes there at the same time (but I'm not there yet).
Hide
Jim Bosch added a comment - - edited

This big ticket is ready for review, and  Andy Salnikov, I'm afraid I need to ask you to be the on to look at most of it (the changes in daf_butler), even though I already asked you to look at the first draft of some of this a week ago.

Christopher Waters, could you take a look at the downstream changes to obs_base, skymap, pipe_base, pipe_tasks, and ci_hsc?  Only the first of those has more than trivial renames, and one specific question I have for you is whether the logic there is big-picture understandable without having to look at the docs for the new DataId object in daf_butler.

Jonathan Sick, if you have time to look at the docstrings in daf.butler.dimensions I'd be grateful; there are a few things (**kwds arguments, plurals of symbols) in particular that I'm not sure I'm handling appropriately.

Show
Jim Bosch added a comment - - edited This big ticket is ready for review, and  Andy Salnikov , I'm afraid I need to ask you to be the on to look at most of it (the changes in daf_butler ), even though I already asked you to look at the first draft of some of this a week ago. Christopher Waters , could you take a look at the downstream changes to obs_base , skymap , pipe_base , pipe_tasks , and ci_hsc ?  Only the first of those has more than trivial renames, and one specific question I have for you is whether the logic there is big-picture understandable without having to look at the docs for the new DataId object in daf_butler. Jonathan Sick , if you have time to look at the docstrings in daf.butler.dimensions I'd be grateful; there are a few things (**kwds arguments, plurals of symbols) in particular that I'm not sure I'm handling appropriately.
Hide
Jim Bosch added a comment -

I'd previously forgotten about pipe_supertask, which also needed some (happily trivial) changes.  Now included as well.

Show
Jim Bosch added a comment - I'd previously forgotten about pipe_supertask , which also needed some (happily trivial) changes.  Now included as well.
Hide
Andy Salnikov added a comment -

I checked daf_butler, looks OK in general and I think I managed to understand how it works  Small bunch of comments on PR. I have also approved pipe_base and pipe_supertask. Removing myself from reviewers, Christopher Waters, please mark it as reviewed once you are done with remaining packages.

Show
Andy Salnikov added a comment - I checked daf_butler , looks OK in general and I think I managed to understand how it works  Small bunch of comments on PR. I have also approved pipe_base and pipe_supertask . Removing myself from reviewers, Christopher Waters , please mark it as reviewed once you are done with remaining packages.
Hide
Christopher Waters added a comment -

I've looked through the set excluding daf_butler (and also enough of daf_butler to convince myself I vaguely know how this works).  I think the obs_base code is generally understandable, although I've added comments on the pull request where I was either confused or unclear.  There are a handful of other small comments (typos and such).

Show
Christopher Waters added a comment - I've looked through the set excluding daf_butler (and also enough of daf_butler to convince myself I vaguely know how this works).  I think the obs_base code is generally understandable, although I've added comments on the pull request where I was either confused or unclear.  There are a handful of other small comments (typos and such).
Hide
Jim Bosch added a comment -

Merged to master.

Thanks, all, for the fast and very helpful reviews.

Show
Jim Bosch added a comment - Merged to master. Thanks, all, for the fast and very helpful reviews.

## People

• Assignee:
Jim Bosch
Reporter:
Jim Bosch
Reviewers:
Christopher Waters
Watchers:
Andy Salnikov, Christopher Waters, Jim Bosch