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

Mapper tests require modification when new datasets are added

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: obs_base
    • Labels:
      None
    • Story Points:
      2
    • Epic Link:
    • Sprint:
      DRP F16-3
    • Team:
      Data Release Production

      Description

      Paul Price recommends a new way to define datasets common to all cameras in daf_butlerUtils, but modifying these yaml files require explicit lists of datasets to be modified in tests/cameraMapper.py.

      If these tests are still useful, they need to depend on a minimal set of dataset definitions instead of the real ones.

        Attachments

          Issue Links

            Activity

            Hide
            tjenness Tim Jenness added a comment -

            Correct. August 17:

            * c7760e9 - (HEAD -> tickets/DM-6858, origin/tickets/DM-6858) testGetDatasetTypes include datasets from base (4 days ago) 
            *   0d2978b - Merge branch 'tickets/DM-7247' (12 days ago) 
            |\  
            | * d1b5bc4 - (origin/tickets/DM-7247, tickets/DM-7247) fix unit test errors & warnings (13 days ago) 
            | * 18e8841 - run stage 2 futurize (13 days ago) 
            | * f9cafd3 - futurize (13 days ago) 
            |/  
            *   e178696 - Merge branch 'tickets/DM-7230' (13 days ago) 
            

            and looking at the tests/testCameraMapper.py on your branch I see:

            testDir = os.path.relpath(os.path.join(getPackageDir('daf_butlerUtils'), 'tests'))
             
            def setup_module(module):
                lsst.utils.tests.init()
             
            class BaseMapper(butlerUtils.CameraMapper):
                packageName = 'base'
             
                def __init__(self):
                    policy = dafPersist.Policy(filePath="tests/BaseMapper.paf")
                    butlerUtils.CameraMapper.__init__(self,
                                                      policy=policy, repositoryDir="tests", root="tests")
                    return
            

            Which sets testDir at the top. Am I looking at the correct branch? Maybe that's why I'm confused. I assumed tickets/DM-6858 was the branch.

            Show
            tjenness Tim Jenness added a comment - Correct. August 17: * c7760e9 - (HEAD -> tickets/DM-6858, origin/tickets/DM-6858) testGetDatasetTypes include datasets from base (4 days ago) * 0d2978b - Merge branch 'tickets/DM-7247' (12 days ago) |\ | * d1b5bc4 - (origin/tickets/DM-7247, tickets/DM-7247) fix unit test errors & warnings (13 days ago) | * 18e8841 - run stage 2 futurize (13 days ago) | * f9cafd3 - futurize (13 days ago) |/ * e178696 - Merge branch 'tickets/DM-7230' (13 days ago) and looking at the tests/testCameraMapper.py on your branch I see: testDir = os.path.relpath(os.path.join(getPackageDir( 'daf_butlerUtils' ), 'tests' ))   def setup_module(module): lsst.utils.tests.init()   class BaseMapper(butlerUtils.CameraMapper): packageName = 'base'   def __init__( self ): policy = dafPersist.Policy(filePath = "tests/BaseMapper.paf" ) butlerUtils.CameraMapper.__init__( self , policy = policy, repositoryDir = "tests" , root = "tests" ) return Which sets testDir at the top. Am I looking at the correct branch? Maybe that's why I'm confused. I assumed tickets/ DM-6858 was the branch.
            Hide
            pgee Perry Gee added a comment -

            Tim Jenness
            Unless it is really important to track this down, couldn't we just make the fix to the existing branch and calll it good? I would have done this anyway.

            I may have later merged to code which had the dataDir change after I was done – I'm really not sure. I did the work on a master which did not have this change.

            Show
            pgee Perry Gee added a comment - Tim Jenness Unless it is really important to track this down, couldn't we just make the fix to the existing branch and calll it good? I would have done this anyway. I may have later merged to code which had the dataDir change after I was done – I'm really not sure. I did the work on a master which did not have this change.
            Hide
            tjenness Tim Jenness added a comment -

            I don't think we are communicating properly. All I'm asking is you to make one tiny edit on your branch to the test file to make it use testDir (that is already defined) rather than bare test. This is one or two lines of change to your existing branch. You don't need to do any complicated merging to sort this out. There is nothing to track down.

            Show
            tjenness Tim Jenness added a comment - I don't think we are communicating properly. All I'm asking is you to make one tiny edit on your branch to the test file to make it use testDir (that is already defined) rather than bare test . This is one or two lines of change to your existing branch. You don't need to do any complicated merging to sort this out. There is nothing to track down.
            Hide
            pgee Perry Gee added a comment -

            Yes, I agree completely. Always have. I thought you were concerned from all your comments about the branch..

            It shall be done as you ask.

            Show
            pgee Perry Gee added a comment - Yes, I agree completely. Always have. I thought you were concerned from all your comments about the branch.. It shall be done as you ask.
            Hide
            pgee Perry Gee added a comment -

            Nate Pease Please confirm changes. Sorry for the confusion about which was the current master.

            Show
            pgee Perry Gee added a comment - Nate Pease Please confirm changes. Sorry for the confusion about which was the current master.

              People

              • Assignee:
                pgee Perry Gee
                Reporter:
                jbosch Jim Bosch
                Reviewers:
                Nate Pease
                Watchers:
                Jim Bosch, Nate Pease, Perry Gee, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Summary Panel