Paul Price added a comment - 23/Jul/15 4:21 PM Lauren MacArthur , would you have a look at this please? It should be pretty straightforward. Russell Owen , your opinions are welcome too. There are three commits on tickets/ DM-3209 of meas_astrom: pprice@tiger-sumire:~/LSST/meas/astrom (tickets/DM-3209=) $ git sub commit 435249afdd611db477909a6eed61d5eaa020ea2a Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:13:41 2015 -0400   showAstrometry: fix prompt We only want the first character, so there's no need to convert the string to a list by using "split", and ["p"] != "p".   python/lsst/meas/astrom/anetAstrometry.py | 1 - 1 file changed, 1 deletion(-)   commit 28eb162eeb1102dabefa1e81538a96e5e79ddbd4 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:17:01 2015 -0400   ANetAstrometryTask: add debugging display of distorted positions Knowing how the positions are being distorted is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetAstrometry.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)   commit a2782e0dedb466b63e4dbd00385892044430fcc4 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:18:11 2015 -0400   ANetBasicAstrometryTask: add debugging display of catalog positions Knowing the positions of catalog objects is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetBasicAstrometry.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)

Russell Owen added a comment - 24/Jul/15 3:06 PM There is already code to display astrometry in display.py (a ds9 version is called displayAstrometry and a matplotlib version called plotAstrometry). Can you just call displayAstrometry? If not, can you refactor the code so the common parts are shared?

Paul Price added a comment - 04/Aug/15 5:24 PM Russell Owen , thanks for pointing out the displayAstrometry function. I've reworked this to use that. It required a few changes to displayAstrometry . Would you please have a look at the revised product? pprice@tiger-sumire:~/LSST/meas/astrom (tickets/DM-3209=) $ git sub commit 801b642bbb5b0d55de199437191efef9e8e953cc Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:13:41 2015 -0400   showAstrometry: fix prompt We only want the first character, so there's no need to convert the string to a list by using "split", and ["p"] != "p".   python/lsst/meas/astrom/anetAstrometry.py | 1 - 1 file changed, 1 deletion(-)   commit aad517439142b1a891f6b7f7261ed58d704cb63d Author: Paul Price <price@astro.princeton.edu> Date: Tue Aug 4 16:13:59 2015 -0400   display: show distorted positions Distorted positions can be displayed by providing the appropriate key for the source catalog.   python/lsst/meas/astrom/display.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)   commit 89ffdaf611c32753d196107dec11722b5c051e89 Author: Paul Price <price@astro.princeton.edu> Date: Tue Aug 4 20:18:40 2015 -0400   display: allow display of sources on existing image This changes the requirement that either an exposure or bbox is provided: if neither is provided, no image is displayed, and we display sources on an existing image display.   python/lsst/meas/astrom/display.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)   commit 34a9be7ca4fb5131e7c8b2577a700473c7c1a818 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:17:01 2015 -0400   ANetAstrometryTask: add debugging display of distorted positions Knowing how the positions are being distorted is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetAstrometry.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)   commit 3f0759ee9c240c523dbeae81ec61ee7ce82a27fe Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:18:11 2015 -0400   ANetBasicAstrometryTask: add debugging display of catalog positions Knowing the positions of catalog objects is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetBasicAstrometry.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+)

Russell Owen added a comment - 05/Aug/15 11:57 AM Thank you for taking advantage of display.displayAstrometry Overall this looks great. I do have a few minor requests. anetAstrometry.py Would you be willing to ditch the existing use of showAstrometry and replace it use with display.displayAstrometry? Or would you be willing to file a ticket for that change? I suggest importing the display module at the beginning, instead of as needed. That will catch a certain class of bugs earlier (instead of only when debugging is enabled, which can be a very confusing time for bugs to show up) and makes dependencies easier to understand. Regarding pausing by breaking into the debugger. I see that I left that out of display.displayAstrometry, and it looks useful. Would you be willing to add that? It will reduce a bit of code duplication and simplify the use of the function. Is it better to just break into the debugger or do something fancier like showAstrometry does? Breaking into the debugger is certainly simpler and saves a step for the user if that is what was wanted. anetBasicAstrometry.py I think you can simplify the new debug code by calling loadPixelBox instead of loadSkyCircle on the refObjLoader (line 858). That sets the centroids for you in the ref catalog (and also trims objects that are off the exposure). When you call displayAstrometry shortly thereafter I suggest you specify the sourceCat and bbox arguments (at present you only specify the refCat). Naturally you can also take advantage of the new pause argument, if you add that. display.py in displayAstrometry please document that distortedCentroidKey can be None

Paul Price added a comment - 05/Aug/15 2:31 PM I've adjusted the code in response to your comments, and pushed. The only items I didn't address are: Removing showAstrometry : I think this is more complicated than I'd like to get into, so I filed DM-3406 . There are actually three high-level display codes in use, so this needs more thought than I can afford to give it. I hope that those thinking about displays (e.g., Robert Lupton and Xiuqin Wu [X] ) will consider this. I deliberately do not provide the sourceCat and bbox arguments when calling displayAstrometry in anetBasicAstrometry.py . If I add bbox , then it will display a blank image. Instead, I want it to display the reference catalog positions over the top of the exposure and source catalog already displayed. I like dropping into the debugger for the pause, as it removes the need for me to do any coding for user interaction, provides an effective pause functionality, and allows the user to poke around. Here's the revised patch set, which I propose to merge in an hour or two: pprice@tiger-sumire:~/LSST/meas/astrom (tickets/DM-3209=) $ git sub commit 801b642bbb5b0d55de199437191efef9e8e953cc Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:13:41 2015 -0400   showAstrometry: fix prompt We only want the first character, so there's no need to convert the string to a list by using "split", and ["p"] != "p".   python/lsst/meas/astrom/anetAstrometry.py | 1 - 1 file changed, 1 deletion(-)   commit 04435c0fb7ad296e3e3b2a60bb552c6cda5c10ca Author: Paul Price <price@astro.princeton.edu> Date: Tue Aug 4 16:13:59 2015 -0400   display: show distorted positions Distorted positions can be displayed by providing the appropriate key for the source catalog.   python/lsst/meas/astrom/display.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)   commit b4be823c94592ee196d0a8e39072cfe27454ac11 Author: Paul Price <price@astro.princeton.edu> Date: Tue Aug 4 20:18:40 2015 -0400   display: allow display of sources on existing image This changes the requirement that either an exposure or bbox is provided: if neither is provided, no image is displayed, and we display sources on an existing image display.   python/lsst/meas/astrom/display.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)   commit 5f105e066f5f47609f34e83dd3587dbe8abc0894 Author: Paul Price <price@astro.princeton.edu> Date: Wed Aug 5 16:48:42 2015 -0400   display: add capability to pause after displaying image This allows the user time to examine the display before another clobbers it. We use pdb.set_trace() as an effective means of pausing, without having to handle user input ourselves; it also allows the user to poke around.   python/lsst/meas/astrom/display.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)   commit 292c9bb726ba3427f3773f3f8b738e6c8e742b1e Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:17:01 2015 -0400   ANetAstrometryTask: add debugging display of distorted positions Knowing how the positions are being distorted is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetAstrometry.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)   commit 8e82fbe0692ffe7f2f8dd22c1962eb315a83c862 Author: Paul Price <price@astro.princeton.edu> Date: Thu Jul 23 19:18:11 2015 -0400   ANetBasicAstrometryTask: add debugging display of catalog positions Knowing the positions of catalog objects is very helpful in debugging astrometric failures.   python/lsst/meas/astrom/anetBasicAstrometry.py | 8 ++++++++ 1 file changed, 8 insertions(+)   commit dae8d234ea625ffd8f7cb24df694a62a1edb77d4 Author: Paul Price <price@astro.princeton.edu> Date: Wed Aug 5 16:52:15 2015 -0400   ANetAstrometryTask: update docs following changes to displays   python/lsst/meas/astrom/anetAstrometry.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)

Paul Price added a comment - 05/Aug/15 4:55 PM Merged to master.