Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-6903

Add an option to label ccd serial number on the showVisitSkyMap.py plot

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: skymap
    • Labels:
      None

      Description

      (actual assignee: Samuel Piehl)

      Sometimes it is useful to know where the CCDs are on the plot. Add an option to label the CCD numbers.

        Attachments

          Activity

          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Here is a plot from Sam, using the new feature.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Here is a plot from Sam, using the new feature.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Lauren, may you please review this small ticket?

          Sam doesn't have an account on Jira, but he can see this page and I can post his comments for him too.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Lauren, may you please review this small ticket? Sam doesn't have an account on Jira, but he can see this page and I can post his comments for him too.
          Hide
          lauren Lauren MacArthur added a comment -

          Sam & Hsin-Fang Chiang: this looks good (having recently added the same feature to my plotting scripts, I certainly appreciate how useful it can be!). I have one comment, though. The figure you show is for 3 widely separated visits, so it looks quite nice. However, for a large number of visits/tighter dithering pattern, labelling every CCD will result in significant clutter. May I suggest adding the ability to add CCD labels to a restricted number of visits? This could be done in many ways:

          • only add labels to one visit (if the data all come from the same instrument, this will likely provide all the info you need to id the ccds on the non-labeled visits)
          • only add labels if the visit is separated from any other labeled visit by some minimum amount
          • etc...!

          If you do opt to label a restricted number of visits, I also like emphasizing (darker shading) of the border for just those visits, e.g. by adding the line

          pyplot.fill(ra, dec, fill=False, alpha=0.4, color=color, edgecolor=color)
          

          when adding the CCD label.

          The above is all aesthetics, so feel free to disregard if you prefer the all-or-nothing approach.

          Finally, is it worth specifying in the command line help message that by "ccd numbers", you are getting what is returned by getSerial() (i.e. you could just add the word "serial"). There is often confusion about what is meant by ccd blank (where blank == number vs. name vs. id vs. serial...).

          Show
          lauren Lauren MacArthur added a comment - Sam & Hsin-Fang Chiang : this looks good (having recently added the same feature to my plotting scripts, I certainly appreciate how useful it can be!). I have one comment, though. The figure you show is for 3 widely separated visits, so it looks quite nice. However, for a large number of visits/tighter dithering pattern, labelling every CCD will result in significant clutter. May I suggest adding the ability to add CCD labels to a restricted number of visits? This could be done in many ways: only add labels to one visit (if the data all come from the same instrument, this will likely provide all the info you need to id the ccds on the non-labeled visits) only add labels if the visit is separated from any other labeled visit by some minimum amount etc...! If you do opt to label a restricted number of visits, I also like emphasizing (darker shading) of the border for just those visits, e.g. by adding the line pyplot.fill(ra, dec, fill=False, alpha=0.4, color=color, edgecolor=color) when adding the CCD label. The above is all aesthetics, so feel free to disregard if you prefer the all-or-nothing approach. Finally, is it worth specifying in the command line help message that by "ccd numbers", you are getting what is returned by getSerial() (i.e. you could just add the word "serial"). There is often confusion about what is meant by ccd blank (where blank == number vs. name vs. id vs. serial...).
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Thank you Lauren MacArthur for your review. They are great suggestions. Sam edited the patch to label one CCD number at one location and ignore the overlapped CCDs. Here is a new plot.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Thank you Lauren MacArthur for your review. They are great suggestions. Sam edited the patch to label one CCD number at one location and ignore the overlapped CCDs. Here is a new plot.
          Hide
          lauren Lauren MacArthur added a comment -

          Hhhmmm....should that "pink" ccd 11 be empty (looking at the previous plot)? Otherwise, looks great!

          Show
          lauren Lauren MacArthur added a comment - Hhhmmm....should that "pink" ccd 11 be empty (looking at the previous plot)? Otherwise, looks great!
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          It looks like in the new plot, he included one more visit which points at roughly same location as the upper left visit (the red and the ~green). The red one doesn't have ccd 11, so it looks pinker than other CCDs.

          Show
          hchiang2 Hsin-Fang Chiang added a comment - It looks like in the new plot, he included one more visit which points at roughly same location as the upper left visit (the red and the ~green). The red one doesn't have ccd 11, so it looks pinker than other CCDs.
          Hide
          lauren Lauren MacArthur added a comment -

          Ah, I see.

          It looks like the indentation needs fixing at line 82, and could you ask Sam to shorten the commit message (should be limited to 50 char: https://developer.lsst.io/processes/workflow.html#git-commit-message-best-practices). Otherwise, feel free to merge.

          Show
          lauren Lauren MacArthur added a comment - Ah, I see. It looks like the indentation needs fixing at line 82, and could you ask Sam to shorten the commit message (should be limited to 50 char: https://developer.lsst.io/processes/workflow.html#git-commit-message-best-practices ). Otherwise, feel free to merge.
          Hide
          hchiang2 Hsin-Fang Chiang added a comment -

          Corrections are done and changes are merged to master.

          Thank you again, Lauren MacArthur!

          Show
          hchiang2 Hsin-Fang Chiang added a comment - Corrections are done and changes are merged to master. Thank you again, Lauren MacArthur !

            People

            • Assignee:
              hchiang2 Hsin-Fang Chiang
              Reporter:
              hchiang2 Hsin-Fang Chiang
              Reviewers:
              Lauren MacArthur
              Watchers:
              Hsin-Fang Chiang, Lauren MacArthur
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Summary Panel