Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Component/s: pipe_tasks
-
Labels:
-
Story Points:1
-
Epic Link:
-
Sprint:AP S19-3
-
Team:Alert Production
Description
There is code in pipe_tasks that uses astrometry_net to load catalogs and fit astrometric solutions. This ticket is to move to the new reference catalog format and newer astrometry fitter.
Note that when this is done in pipe_tasks that we can remove meas_extensions_astrometryNet as a dependency.
I suggest this wait on DM-2186, though that is not strictly necessary.
Attachments
Issue Links
- blocks
-
DM-18034 Remove astrometry.net eups table dependencies
- Invalid
- is blocked by
-
- Done
- is triggering
-
- Done
- relates to
-
DM-18394 Crash when running ap_pipe on calexp templates
- Done
-
- Done
-
- Withdrawn
-
- Done
-
- Implemented
- mentioned in
-
Page Loading...
Activity
A test is being removed from pipe_tasks that isn't being reinstated somewhere. I am concerned that this is violating Robert Lupton's requirement "that we keep astrometry.net alive in reality": after DM-18036, there won't be anything that actually exercises the astrometry.net matcher under ProcessCcdTask.
I find the commit messages to be lacking. Most contain the text "Remove no longer necessary retarget", which doesn't read well, and does not allow someone perusing the history to easily understand what's going on. I would prefer to see commit messages that say something like:
config: remove retarget to LoadIndexedReferenceObjectsTask
|
|
|
It is now the default setting.
|
That has multiple advantages over what's present now:
1. It specifies the area of the codebase that's being changed (config).
2. It specifies exactly what is being changed (LoadIndexedReferenceObjectsTask).
3. It specifies why it's being changed (it's now the default).
You may think this is overkill for such a simple ticket, but it's important to maintain the habit of providing that information for all tickets.
Paul Price
added a comment - A test is being removed from pipe_tasks that isn't being reinstated somewhere. I am concerned that this is violating Robert Lupton 's requirement "that we keep astrometry.net alive in reality": after DM-18036 , there won't be anything that actually exercises the astrometry.net matcher under ProcessCcdTask .
I find the commit messages to be lacking. Most contain the text "Remove no longer necessary retarget", which doesn't read well, and does not allow someone perusing the history to easily understand what's going on. I would prefer to see commit messages that say something like:
config: remove retarget to LoadIndexedReferenceObjectsTask
It is now the default setting.
That has multiple advantages over what's present now:
1. It specifies the area of the codebase that's being changed ( config ).
2. It specifies exactly what is being changed ( LoadIndexedReferenceObjectsTask ).
3. It specifies why it's being changed (it's now the default).
You may think this is overkill for such a simple ticket, but it's important to maintain the habit of providing that information for all tickets. Note that RFC-562 was adopted: a.net will not be supported by gen3 middleware.
Fair point on the commit messages: I've updated the obs packages to something similar to the one you give above.
Getting to the point where no code run by CI exercises the a.net matcher is a requirement for implementing RFC-562. I think the stack demo is the last bit of CI code that uses a.net refcats.
Thank you for the quick review: the obs package commit messages should be better now.
Merged and done.
Jenkins passed (after some mucking with the demo)!
Paul Price: are you able to review this ticket (-800 or so lines, +20)? It's 5 obs packages, the demo, and pipe_tasks. Jenkins passed including ci_hsc, but I'm not sure where else to check.
pipe_tasks PR, since Jira isn't picking it up: https://github.com/lsst/pipe_tasks/pull/278