Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-23975

Subfilter type inconsistency

    Details

    • Story Points:
      4
    • Sprint:
      AP S20-4 (March)
    • Team:
      Alert Production
    • Urgent?:
      No

      Description

      When working on DM-23778 I realized that in obs_base and obs_decam we had defined subfilter to be an int but in obs_lsst and gen3 butler it was defined to be a string.

      To be explicit here. obs_base uses:

      %(filter)s%(subfilter)dof%(numSubfilters)d/
      

      obs_decam uses:

      %(filter)s%(subfilter)d
      

      but obs_lsst was using:

      %(filter)s%(subfilter)s
      

      I had to change obs_lsst to an int to prevent the 2to3 conversion code from complaining about the inconsistency but I worry that I changed the wrong end and should have changed obs_base.

      Given that the gen3 data model uses a string I feel thes gen2 templates need to be looked at and made consistent with gen3.

        Attachments

          Issue Links

            Activity

            Hide
            sullivan Ian Sullivan added a comment -

            I'm sorry, I thought I had fixed this before. The subfilter should be an int in all cases, so I will make branches with this ticket number and make it consistent.

            Show
            sullivan Ian Sullivan added a comment - I'm sorry, I thought I had fixed this before. The subfilter should be an int in all cases, so I will make branches with this ticket number and make it consistent.
            Hide
            tjenness Tim Jenness added a comment -

            If it's always an int then I think the only place that needs fixing is the gen3 dimension definition in daf_butler.

            Show
            tjenness Tim Jenness added a comment - If it's always an int then I think the only place that needs fixing is the gen3 dimension definition in daf_butler.
            Hide
            sullivan Ian Sullivan added a comment -

            Ah, I misread the ticket, and I see you already made the change. The changes to obs_lsst in DM-22605 look correct, thank you. Aside from confirming that this was the correct change, is anything more needed on this ticket Tim Jenness?

            Show
            sullivan Ian Sullivan added a comment - Ah, I misread the ticket, and I see you already made the change. The changes to obs_lsst in DM-22605 look correct, thank you. Aside from confirming that this was the correct change, is anything more needed on this ticket Tim Jenness ?
            Hide
            tjenness Tim Jenness added a comment -

            I think daf_butler is the only place you need to fix anything.

            Show
            tjenness Tim Jenness added a comment - I think daf_butler is the only place you need to fix anything.
            Hide
            sullivan Ian Sullivan added a comment -

            After further investigation I determined that the subfilter dimension in Gen 3 needs to be a string, so I have changed the Gen 2 obs packages to be consistent with the Gen 3 definition. Please note that there is a ticket branch for daf_butler that I made initially, but it contains no actual changes and will not be merged. 

            The pull requests containing actual changes are below:

            https://github.com/lsst/obs_base/pull/211

            https://github.com/lsst/obs_decam/pull/134

            https://github.com/lsst/obs_lsst/pull/190

             

            Show
            sullivan Ian Sullivan added a comment - After further investigation I determined that the subfilter dimension in Gen 3 needs to be a string, so I have changed the Gen 2 obs packages to be consistent with the Gen 3 definition. Please note that there is a ticket branch for daf_butler that I made initially, but it contains no actual changes and will not be merged.  The pull requests containing actual changes are below: https://github.com/lsst/obs_base/pull/211 https://github.com/lsst/obs_decam/pull/134 https://github.com/lsst/obs_lsst/pull/190  
            Hide
            tjenness Tim Jenness added a comment -

            We agreed with Jim Bosch that the changes to daf_butler were sufficient and the obs packages did not need to be changed. The daf_butler changes have been reviewed and we believe that the name is not necessary.

            Show
            tjenness Tim Jenness added a comment - We agreed with Jim Bosch that the changes to daf_butler were sufficient and the obs packages did not need to be changed. The daf_butler changes have been reviewed and we believe that the name is not necessary.
            Hide
            sullivan Ian Sullivan added a comment -

            I've removed the name, since I could verify that it was not needed. Unfortunately I also needed to make a few changes in pipe_tasks, so there is one more pull request. I am not planning to merge any of the changes to the obs packages.

            Show
            sullivan Ian Sullivan added a comment - I've removed the name, since I could verify that it was not needed. Unfortunately I also needed to make a few changes in pipe_tasks , so there is one more pull request. I am not planning to merge any of the changes to the obs packages.
            Hide
            sullivan Ian Sullivan added a comment -

            Story points updated to reflect effort to fix additional bugs that had to be fixed to test the Gen 3 changes.

            Show
            sullivan Ian Sullivan added a comment - Story points updated to reflect effort to fix additional bugs that had to be fixed to test the Gen 3 changes.

              People

              • Assignee:
                sullivan Ian Sullivan
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Tim Jenness
                Watchers:
                Ian Sullivan, Jim Bosch, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel