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

Convert getTargetList to a classmethod

    Details

      Description

      Inheritance of `getTargetList` would be simplified if it were a `classmethod` instead of a `staticmethod`. This ticket is to write a trial implementation to see if a change is feasible in a short amount of time. If it takes too long, I will stick with my existing workarounds in `pipe_tasks.dcrMultiBand.py`.

        Attachments

          Activity

          Hide
          sullivan Ian Sullivan added a comment -

          In addition to the linked pull requests, there is also one for `pipe_tasks`. This review should hopefully be straightforward since it mostly just changes the same few lines in every package in `lsst_distrib` I could find that uses `getTargetList`. Please let me know if there are any consequences of this change you think I may not be aware of.

          Show
          sullivan Ian Sullivan added a comment - In addition to the linked pull requests, there is also one for `pipe_tasks`. This review should hopefully be straightforward since it mostly just changes the same few lines in every package in `lsst_distrib` I could find that uses `getTargetList`. Please let me know if there are any consequences of this change you think I may not be aware of.
          Hide
          rowen Russell Owen added a comment - - edited

          In many cases when you changed getTargetList to a classmethod you changed the body to call cls.getTargetList(...) instead of <parent_class>.getTargetList(...), which I think should cause an infinite loop. If this works I am very puzzled why. In any case, I think all such changes should either be reverted or changed to super().getTargetList(...)

          There is a small benefit to one task in pipe_tasks, which is an existence proof that this change is useful. On the other hand, the improvement is fairly small and the cost is rather high in the number of packages that need editing. I lean towards dropping this ticket but am happy to approve it if you fix the calls to cls.getTargetList.

          Show
          rowen Russell Owen added a comment - - edited In many cases when you changed getTargetList to a classmethod you changed the body to call cls.getTargetList(...) instead of <parent_class>.getTargetList(...) , which I think should cause an infinite loop. If this works I am very puzzled why. In any case, I think all such changes should either be reverted or changed to super().getTargetList(...) There is a small benefit to one task in pipe_tasks, which is an existence proof that this change is useful. On the other hand, the improvement is fairly small and the cost is rather high in the number of packages that need editing. I lean towards dropping this ticket but am happy to approve it if you fix the calls to cls.getTargetList .
          Hide
          sullivan Ian Sullivan added a comment -

          This fails on Jenkins even after making Russell Owen's suggested changes. I've copied the error below for posterity, but I'm going to close this for now with the assumption that it will become obsolete once we switch over to the new PipelineTask framework.

          Error
          AssertionError: CFHT Quick Test failed
          Stacktrace
          self = <test_obs.ExampleObsTestCase testMethod=testObsCfhtQuick>
          def testObsCfhtQuick(self):
          """Test obs_cfht"""
          self.assertExecutable("runCfhtQuickTest.sh",
          root_dir=executable_dir,
          args=["--", "--noplot"],
          > msg="CFHT Quick Test failed")
          tests/test_obs.py:43:
          _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
          ../../stack/Linux64/utils/16.0-6-g3610b4f+3/python/lsst/utils/tests.py:297: in assertExecutable
          self.fail(msg)
          E AssertionError: CFHT Quick Test failed
          Standard Output
          Running executable 'scripts/runCfhtQuickTest.sh' with arguments "-- --noplot"...
          Ingesting Raw data
          root INFO: Loading config overrride file '/j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/obs_cfht/16.0-2-gd82908a+32/config/ingest.py'
          CameraMapper INFO: Loading Posix exposure registry from /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input
          ingest INFO: /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/validation_data_cfht/16.0+38/raw/849375p.fits.fz --<link>--> /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input/raw/06AL01/D3/2006-05-20/r/849375p.fits.fz
          ingest INFO: /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/validation_data_cfht/16.0+38/raw/850587p.fits.fz --<link>--> /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input/raw/06AL01/D3/2006-06-02/r/850587p.fits.fz
          running singleFrameDriver
          Standard Error
          /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.6/site-packages/astropy/config/configuration.py:541: ConfigurationMissingWarning: Configuration defaults will be used due to OSError:Could not find unix home directory to search for astropy config dir on None
          warn(ConfigurationMissingWarning(msg))
          /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.6/site-packages/astropy/config/configuration.py:541: ConfigurationMissingWarning: Configuration defaults will be used due to OSError:Could not find unix home directory to search for astropy config dir on None
          warn(ConfigurationMissingWarning(msg))
          

          Show
          sullivan Ian Sullivan added a comment - This fails on Jenkins even after making Russell Owen 's suggested changes. I've copied the error below for posterity, but I'm going to close this for now with the assumption that it will become obsolete once we switch over to the new PipelineTask framework. Error AssertionError: CFHT Quick Test failed Stacktrace self = <test_obs.ExampleObsTestCase testMethod=testObsCfhtQuick> def testObsCfhtQuick(self): """Test obs_cfht""" self.assertExecutable("runCfhtQuickTest.sh", root_dir=executable_dir, args=["--", "--noplot"], > msg="CFHT Quick Test failed") tests/test_obs.py:43: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../stack/Linux64/utils/16.0-6-g3610b4f+3/python/lsst/utils/tests.py:297: in assertExecutable self.fail(msg) E AssertionError: CFHT Quick Test failed Standard Output Running executable 'scripts/runCfhtQuickTest.sh' with arguments "-- --noplot"... Ingesting Raw data root INFO: Loading config overrride file '/j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/obs_cfht/16.0-2-gd82908a+32/config/ingest.py' CameraMapper INFO: Loading Posix exposure registry from /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input ingest INFO: /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/validation_data_cfht/16.0+38/raw/849375p.fits.fz --<link>--> /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input/raw/06AL01/D3/2006-05-20/r/849375p.fits.fz ingest INFO: /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/stack/Linux64/validation_data_cfht/16.0+38/raw/850587p.fits.fz --<link>--> /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/build/lsst_ci/CfhtQuick/input/raw/06AL01/D3/2006-06-02/r/850587p.fits.fz running singleFrameDriver Standard Error /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.6/site-packages/astropy/config/configuration.py:541: ConfigurationMissingWarning: Configuration defaults will be used due to OSError:Could not find unix home directory to search for astropy config dir on None warn(ConfigurationMissingWarning(msg)) /j/ws/stack-os-matrix/centos-7.devtoolset-6.py3/lsstsw/miniconda/envs/lsst-scipipe/lib/python3.6/site-packages/astropy/config/configuration.py:541: ConfigurationMissingWarning: Configuration defaults will be used due to OSError:Could not find unix home directory to search for astropy config dir on None warn(ConfigurationMissingWarning(msg))

            People

            • Assignee:
              sullivan Ian Sullivan
              Reporter:
              sullivan Ian Sullivan
              Reviewers:
              Russell Owen
              Watchers:
              Ian Sullivan, John Swinbank, Russell Owen
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel