Review for pipe_tasks in DM-2939: modernize astrometry interfaces #15
Conversation
| astrometer = pexConfig.ConfigurableField( | ||
| target = measAstrom.AstrometryTask, | ||
| doc = "astrometry task; used to match sources to reference objects, but not to fit a WCS", | ||
| ) |
There was a problem hiding this comment.
It looks like you're touching all the places where the astrometry subtask is used here; could we rename it at the same time to astrometry for consistency?
There are also other places we use astrometer, which I'd also like to change; I prefer just calling the subtask astrometry, since we tend to name other tasks after the "act of doing something" (e.g. measurement) not "a thing that does something" (measurer).
There was a problem hiding this comment.
OK. Done in pipe_tasks, and that exposed one instance where I had failed to update an interface. obs_subaru has an instance of a class that has an astrometer subtask and an astrometry method; that will take some thought.
There was a problem hiding this comment.
Do recall what class that was? I think now obs_subaru is now integrated into the stack enough we should at least just consider changing it here.
There was a problem hiding this comment.
It's for using hscAstrom instead of meas_astrom. That shouldn't be necessary any more. Don't worry about it. If we need it for HSC, I'll work around you.
There was a problem hiding this comment.
The task in question is SubaruAstrometryTask and is still present on master. I think it does require need updating because some of the code it used from meas_algorithms changed. For example SubaruAstrometryTask is a subtask of what was AstrometryTask but became ANetAstrometryTask. I have updated it on tickets/DM-2939. I removed the term "astrometer" at the same time (it was a very minor change).
r-owen
force-pushed
the
tickets/DM-2939
branch
from
bdf2fff
to
c985b87
Compare
ImageDifferenceTask used outdated code to find reference objects that overlap an exposure and match them to sources. I updated the code to use the modern astrometry task API.
I improved the documentation of two values returned by CalibrateTask.run.
MeasureMergedCoaddSourcesTask now calls the astrometry tasks's run method instead of assuming it has an astrometer which has a useKnownWcs method.
For consistency rename a local variable astrometer to astrometry in ProcessImageTask
ImageDifferenceTask had a workaround for the fact that our tan-sip fitter used to fail if CRPIX was large. That's been fixed, so I removed the workaround and managed to eliminate use of a hard-coded astrometry task at the same time.
r-owen
force-pushed
the
tickets/DM-2939
branch
from
c985b87
to
001f098
Compare
No description provided.