# Stop using astrometry_net by default

XMLWordPrintable

## Details

• Type: Improvement
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Sprint:
AP S19-3
• Team:

## 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.

## Activity

Hide
John Parejko added a comment -

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.

Show
John Parejko added a comment - 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
Hide
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).
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.

Show
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.
Hide
John Parejko added a comment -

Note that RFC-562 was adopted: a.net will not be supported by gen3 middleware.

Show
John Parejko added a comment - Note that RFC-562 was adopted: a.net will not be supported by gen3 middleware.
Hide
John Parejko added a comment - - edited

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.

Show
John Parejko added a comment - - edited 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.
Hide
John Parejko added a comment -

Thank you for the quick review: the obs package commit messages should be better now.

Merged and done.

Show
John Parejko added a comment - Thank you for the quick review: the obs package commit messages should be better now. Merged and done.

## People

• Assignee:
John Parejko
Reporter:
Russell Owen
Reviewers:
Paul Price
Watchers:
Hsin-Fang Chiang, John Parejko, Paul Price, Robert Lupton, Russell Owen, Simon Krughoff, Yusra AlSayyad