DM-36403: Add utility function for summing footprint fluxes #21
Conversation
mfisherlevine
force-pushed
the
tickets/DM-36403
branch
from
0abce60
to
983f5b5
Compare
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
| 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
| @@ -207,6 +213,59 @@ def test_quantiles(self): | |||
| np.testing.assert_almost_equal(edges1, edges2, decimal=decimal) | |||
|
|
|||
|
|
|||
| 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
No description provided.