Details
-
Type:
Story
-
Status: Won't Fix
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: ctrl_pool, jointcal, meas_algorithms, meas_base, meas_mosaic, pipe_base, pipe_drivers, pipe_tasks, validate_drp
-
Labels:
-
Story Points:2
-
Epic Link:
-
Sprint:AP F18-3
-
Team:Alert Production
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
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.
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))
|
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.