# Subfilter type inconsistency

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
4
• Sprint:
AP S20-4 (March)
• Team:
• 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.

## Activity

Hide
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
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
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
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
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
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
Tim Jenness added a comment -

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

Show
Tim Jenness added a comment - I think daf_butler is the only place you need to fix anything.
Hide
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
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
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
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
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
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
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
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:
Ian Sullivan
Reporter:
Tim Jenness
Reviewers:
Tim Jenness
Watchers:
Ian Sullivan, Jim Bosch, Tim Jenness