DM-36403: Add utility function for summing footprint fluxes#21
Conversation
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
0abce60 to
983f5b5
Compare
parejkoj
left a comment
parejkoj
left a comment
There was a problem hiding this comment.
I think this could reasonably go into afw/detection/utils.py Any reason why you wouldn't want it here?
tests/test_utils.py
Outdated
|
|
||
| def test_fluxFromFootprint(self): | ||
| footPrintSet = detectObjectsInExp(self.exp) | ||
| allFootprints = footPrintSet.getFootprints() |
There was a problem hiding this comment.
This test doesn't need a butler at all, you could construct the image directly. For example, afw/tests/test_footprint[1,2].py place objects in an Image and make the footprints in place.
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
2 times, most recently
from
4bba683 to
7ff741c
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
2 times, most recently
from
4e52d9b to
c6fe637
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
c6fe637 to
dc7af54
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
dc7af54 to
0d87de1
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
0d87de1 to
d74333b
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
d74333b to
54e891a
Compare
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
0525d6a to
458c948
Compare
parejkoj
left a comment
parejkoj
left a comment
There was a problem hiding this comment.
Handful of comments, most important being that you need to change extract to compute when calling the afw function.
| Returns | ||
| ------- | ||
| fluxes : `list` of `float` | ||
| The fluxes for each footprint. |
There was a problem hiding this comment.
Is there any value in returning a numpy array here instead?
There was a problem hiding this comment.
I'm happy with a list, personally. If the user needs, it can always be cast with np.asarray as needed.
There was a problem hiding this comment.
I can't see the harm. I'll implement unless @mfisherlevine objects.
tests/test_utils.py
Outdated
|
|
||
|
|
||
| class ImageBasedTestCase(lsst.utils.tests.TestCase): | ||
| """A test case for testing sky position offsets for exposures.""" |
There was a problem hiding this comment.
Are the docstring and/or class name right?
There was a problem hiding this comment.
Agree, docstring looks wrong. I'm going to leave fixing these to @fred3m though to save having too many cooks here.
There was a problem hiding this comment.
Unless either of you has a suggestion, I'm just going to delete the docstring. Most of the other test classes don't have docstrings, and since it currently only contains one test, I'm not sure what the long intent of this class is.
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
856f445 to
287fe20
Compare
3 participants
No description provided.