DM-26783: DiaForcedSource on associated DiaObject off frame #99
Conversation
tests/test_diaForcedSource.py
Outdated
| @@ -37,7 +37,9 @@ | |||
|
|
|||
|
|
|||
| def create_test_dia_objects(n_points, wcs, startPos=100): | |||
| """Create dummy DIASources or DIAObjects for use in our tests. | |||
| """Create dummy DIASources or DIAObjects for use in our tests. Adds | |||
| Three test sources outsize of the ccd area. | |||
There was a problem hiding this comment.
"outside" of the ccd area?
There was a problem hiding this comment.
Moved them to the setUp section and added a bit more clarity in the comments.
| """Create dummy DIASources or DIAObjects for use in our tests. | ||
| """Create dummy DIASources or DIAObjects for use in our tests. Adds | ||
| Three test sources outsize of the ccd area. | ||
|
|
There was a problem hiding this comment.
Not in the currently changed code, but I notice below that two of the outside sources are given src['id'] = 10000001. Is that a bug?
There was a problem hiding this comment.
Thanks for the find, fixed.
tests/test_diaForcedSource.py
Outdated
| @@ -202,7 +211,7 @@ def testRun(self): | |||
|
|
|||
| # Should be number of test objects minus one as one object is purposely | |||
| # outside of the ccd area. | |||
| self.assertEqual(len(dia_forced_sources), len(self.testDiaObjects) - 3) | |||
| self.assertEqual(len(dia_forced_sources), len(self.testDiaObjects) - 2) | |||
There was a problem hiding this comment.
I'm confused about this line and the comment. It looks like you're adding 3 sources outside the ccd in create_test_dia_objects, yet the comment says one and the comparison here looks like two.
After playing with the code a little, it looks like the selection of five sources on line 193 is including two of the three outside sources, and omitting the third. If I expand that to [1:7] toinclude 6 sources and also fix the source id of the third outside source, then this line needs to be len(self.testDiaObjects) - 1 to pass, which matches the comment. I'm pretty sure I'm missing something here, though. Could you please double-check the logic, and either expand the comment to explain why there is a -2 here, or modify the code if it is incorrect in some way?
There was a problem hiding this comment.
Made a variable in setup and added more comments for clarity (hopefully).
Debug code and tests. Fix column name in diaForcedSourceTask. Fix linting. Fix up unittest.
morriscb
force-pushed
the
tickets/DM-26783
branch
from
8f00a89
to
aa9be45
Compare
tests/test_diaForcedSource.py
Outdated
| # processing. | ||
| self.updatedTestIds = np.array([1, 2, 3, 4, 10000001], dtype=np.uint64) | ||
| # Expecdted number of sources is the number of updated ids plus | ||
| # plus any that are within the CCD footprint but are not in the |
Debug unittest. Fix typo
morriscb
force-pushed
the
tickets/DM-26783
branch
from
aa9be45
to
c403758
Compare
No description provided.