Details
-
Type:
Story
-
Status: Invalid
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: meas_algorithms
-
Labels:
-
Urgent?:No
Description
The gen3 refloader has no real unittests, and is known to be missing gen2 features (e.g. DM-27843, DM-28991, DM-27921). The gen2 refloader LoadReferenceObjectsTask has a lot of tests built around the indexed refloader, but they rely on gen2 butler infrastructure and test data. The gen3 refloader, ReferenceObjectLoader, is explicitly only an indexed one, so we should be able to use some or most of the existing tests for it.
When I've talked with Nate Lust about this, he's said that a ground-up rewrite of the gen3 loader was something he's wanted to do, but I don't know if that's the best approach, or if we should just try to get as much code into the shared base class, ReferenceObjectLoaderBase as possible and then figure out how to make the existing tests work in gen3. How the gen2 code is spread between the LoadReferenceObjectsTask base class and the LoadIndexedReferenceObjectsTask also makes this tricky, due to the legacy of the astrometry.net refloader.
Attachments
Issue Links
- relates to
-
DM-27843 add anyFilterMapsToThis support to ReferenceObjectLoader
- Done
-
- To Do
-
- Done
-
- Won't Fix
-
- Done
Activity
I did a lot of work related to this on DM-27843 (refactoring things into a common base class, and then testing that base class). I haven't looked at the test coverage since Eli's gen2 removal. If the 2-3 refcat tests in meas_algorithms result in good coverage for ReferenceObjectLoader, then I think we can close this.
Current coverage is:
python/lsst/meas/algorithms/__init__.py 49 0 0 0 100%
|
python/lsst/meas/algorithms/accumulator_mean_stack.py 81 0 34 3 97%
|
python/lsst/meas/algorithms/astrometrySourceSelector.py 58 1 14 0 96%
|
python/lsst/meas/algorithms/brightStarStamps.py 140 14 56 9 87%
|
python/lsst/meas/algorithms/coaddPsf/__init__.py 2 0 0 0 100%
|
python/lsst/meas/algorithms/coaddPsf/coaddPsfContinued.py 4 0 0 0 100%
|
python/lsst/meas/algorithms/convertRefcatManager.py 159 68 60 8 55%
|
python/lsst/meas/algorithms/convertReferenceCatalog.py 62 1 12 1 97%
|
python/lsst/meas/algorithms/detection.py 316 45 130 17 82%
|
python/lsst/meas/algorithms/dynamicDetection.py 109 3 24 6 93%
|
python/lsst/meas/algorithms/findCosmicRaysConfig.py 18 0 2 0 100%
|
python/lsst/meas/algorithms/flaggedSourceSelector.py 15 0 4 0 100%
|
python/lsst/meas/algorithms/gaussianPsfFactory.py 44 1 14 1 97%
|
python/lsst/meas/algorithms/htmIndexer.py 17 8 6 0 48%
|
python/lsst/meas/algorithms/indexerRegistry.py 10 0 2 0 100%
|
python/lsst/meas/algorithms/ingestIndexReferenceTask.py 139 14 32 5 88%
|
python/lsst/meas/algorithms/installGaussianPsf.py 27 1 10 2 92%
|
python/lsst/meas/algorithms/loadIndexedReferenceObjects.py 11 0 4 0 100%
|
python/lsst/meas/algorithms/loadReferenceObjects.py 379 78 156 19 79%
|
python/lsst/meas/algorithms/makeCoaddApCorrMap.py 21 1 10 1 94%
|
python/lsst/meas/algorithms/makePsfCandidates.py 45 1 14 1 97%
|
python/lsst/meas/algorithms/matcherSourceSelector.py 47 0 10 1 98%
|
python/lsst/meas/algorithms/measureApCorr.py 122 28 48 4 78%
|
python/lsst/meas/algorithms/objectSizeStarSelector.py 253 96 112 9 61%
|
python/lsst/meas/algorithms/pcaPsfDeterminer.py 338 136 189 24 57%
|
python/lsst/meas/algorithms/psfCandidate/__init__.py 2 0 0 0 100%
|
python/lsst/meas/algorithms/psfCandidate/psfCandidateContinued.py 8 1 2 0 90%
|
python/lsst/meas/algorithms/psfDeterminer.py 18 1 4 0 95%
|
python/lsst/meas/algorithms/readFitsCatalogTask.py 22 1 12 1 94%
|
python/lsst/meas/algorithms/readTextCatalogTask.py 20 0 6 0 100%
|
python/lsst/meas/algorithms/reserveSourcesTask.py 44 0 14 0 100%
|
python/lsst/meas/algorithms/scaleVariance.py 66 45 16 0 30%
|
python/lsst/meas/algorithms/simple_curve.py 154 17 44 9 87%
|
python/lsst/meas/algorithms/skyObjects.py 48 2 16 4 91%
|
python/lsst/meas/algorithms/sourceSelector.py 186 4 82 9 95%
|
python/lsst/meas/algorithms/stamps.py 172 19 70 7 88%
|
python/lsst/meas/algorithms/starSelector.py 25 9 8 0 61%
|
python/lsst/meas/algorithms/subtractBackground.py 113 32 50 13 64%
|
python/lsst/meas/algorithms/testUtils.py 100 1 22 2 98%
|
python/lsst/meas/algorithms/utils.py 545 431 212 15 19%
|
which looks like 79% for the file with the reference object loader in it.
Coverage of just the pytest tests:
Name Stmts Miss Branch BrPart Cover
|
TOTAL 8798 1390 2617 259 82%
|
coverage including the nopytest long test:
TOTAL 8931 1345 2633 267 82%
|
The long test is 133 lines and only misses 2 so the nopytest test is only testing about 30 extra lines in the package. I wonder whether that "nopytest" test is really worth it any more.
My main comment though is whether this ticket should now be blocking a declaration that we have removed gen2.
It's not just the number of lines covered, it's what those lines actually do. That nopytest test is the only integration test of the refcat convert code, so it's absolutely necessary to keep. I need to dig into the full coverage reports on individual tests to ensure that it's still testing what it should be testing.
As to whether this should block the declaration that gen2 is removed, I guess we can remove that as a blocker now. I've marked DM-31698 as related to this one, and will try to do it in this next month: that will help me understand whether we lost essential test coverage. There are still some vestigial gen2 things in meas_algorithms related to this that I will try to clean up there. I don't want to close this ticket until I've confirmed that the convert and refcat code is properly covered.
Marking this invalid, as this work was done on a variety of other tickets as part of the gen2->gen3 conversion and subsequent cleanup.
This ticket indicates that it is blocking the gen2 middleware removal but we've now removed all the gen2 code (effectively, verify is being reviewed). Is it really a blocker to declaring gen2 removal?