DM-27106: Add array-based operations to simple photoCalib routines. #630
Conversation
There was a problem hiding this comment.
Thank you for using astropy units for magnitudes and fluxes here. The more places we can get rid of people hard-coding 3631, the better!
My only substantial objection is the usual one about making the internals of the PhotoCalib object visible in the public API. It's unfortunate that we've built up an API in afw that is now not going to be used much since most people (even within DM) are going to pandas/parquet tables.
| Parameters | ||
| ---------- | ||
| instFluxes : `np.ndarray` (N,) | ||
| Array of instFluxes to convert. |
There was a problem hiding this comment.
Note that instFluxes have units of counts?
| Parameters | ||
| ---------- | ||
| x : `np.ndarray` (N,) | ||
| Array of x values. |
There was a problem hiding this comment.
Should all these x/y parameters say they are in pixels? Should we also say that in the method docstrings (where you say "for numpy arrays")?
tests/test_photoCalib.py
Outdated
| pointXShiftXArray, | ||
| pointXShiftYArray | ||
| ) | ||
| np.testing.assert_array_equal(localCalib2, localCalib) |
There was a problem hiding this comment.
Do we want explicit equality here? We do have self.assertFloatsEqual() for that.
erykoff
force-pushed
the
tickets/DM-27106
branch
from
998eb60
to
ca9ea09
Compare
erykoff
force-pushed
the
tickets/DM-27106
branch
from
ca9ea09
to
166cd07
Compare
This also adds a property to know if the given photoCalib is constant (spatially-varying) or not.