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

Implement putting of matplotlib figures

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: daf_persistence
    • Labels:
      None
    • Story Points:
      1
    • Sprint:
      DRP S18-6
    • Team:
      Data Release Production

      Description

      Make matplotlib figures {{butler.put()}}able.

      Jim Bosch says on Slack:
      > Basically, that involves grepping daf_persistence for FitsCatalogStorage, copying what you see and calling it MatplotlibStorage, and then adjusting it appropriately (i.e. call savefig instead of writeFits, raise an exception when trying to read).

      Paul Price notes that there are some gotchas involving the fact that the backend has to be set immediately after import, and this could prove tricky, especially if people want to combine this with pop-up style debug plots.. Some relevant info on this might be found in DM-14159.

        Attachments

          Activity

          Hide
          jbosch Jim Bosch added a comment -

          This is ready for review. It's a tiny amount of code, but I'd like all three reviewers to look at it from different angles:

          • Merlin Fisher-Levine: does this do everything you need? You mentioned something earlier about wanting to be able to add something to the filename, and while I think that's probably best handled by adding custom data ID keys to the template, there could certainly be something about your use case that I'm missing.
          • Michael Wood-Vasey: could you check that the import and usage of matplotlib matches our current recommended patterns? I think I've done it all safely, and the important code doesn't need to import matplotlib itself at all, but I haven't been following those issues closely and I know they've been tricky.
          • Lauren MacArthur: could you let me know if this looks like something you could use within pipe_analysis, or if there are any additional features I'd need to support for that? (No need to implement or even test it now; I just want to know if I'm missing something that would be obvious to someone familiar with the pipe_analysis codebase).
          Show
          jbosch Jim Bosch added a comment - This is ready for review. It's a tiny amount of code, but I'd like all three reviewers to look at it from different angles: Merlin Fisher-Levine : does this do everything you need? You mentioned something earlier about wanting to be able to add something to the filename, and while I think that's probably best handled by adding custom data ID keys to the template, there could certainly be something about your use case that I'm missing. Michael Wood-Vasey : could you check that the import and usage of matplotlib matches our current recommended patterns? I think I've done it all safely, and the important code doesn't need to import matplotlib itself at all, but I haven't been following those issues closely and I know they've been tricky. Lauren MacArthur : could you let me know if this looks like something you could use within pipe_analysis, or if there are any additional features I'd need to support for that? (No need to implement or even test it now; I just want to know if I'm missing something that would be obvious to someone familiar with the pipe_analysis codebase).
          Hide
          mfisherlevine Merlin Fisher-Levine added a comment -

          Sure, I have to do a gnarly rebase first, as I've fallen way behind the cutting edge, but will do that tonight/tomorrow, and will run this as soon as I'm done and report back.

          And yes, I agree that I will put some extra keys I need in the template and manipulate the dataIds on the way in, such that the butler does all the putting for me correctly.

          Show
          mfisherlevine Merlin Fisher-Levine added a comment - Sure, I have to do a gnarly rebase first, as I've fallen way behind the cutting edge, but will do that tonight/tomorrow, and will run this as soon as I'm done and report back. And yes, I agree that I will put some extra keys I need in the template and manipulate the dataIds on the way in, such that the butler does all the putting for me correctly.
          Hide
          mfisherlevine Merlin Fisher-Levine added a comment - - edited

          I've tested and it's good to go as far as I'm concerned. I'd have marked as "Review complete" but I don't know how that's supposed to go for multiple reviewers, but it's a from me

          Show
          mfisherlevine Merlin Fisher-Levine added a comment - - edited I've tested and it's good to go as far as I'm concerned. I'd have marked as "Review complete" but I don't know how that's supposed to go for multiple reviewers, but it's a from me
          Hide
          jbosch Jim Bosch added a comment -

          Michael Wood-Vasey: ping (I'm guessing you didn't notice this or forgot about it).

          Show
          jbosch Jim Bosch added a comment - Michael Wood-Vasey : ping (I'm guessing you didn't notice this or forgot about it).
          Hide
          wmwood-vasey Michael Wood-Vasey added a comment -

          Thanks for the ping, Jim Bosch This came to me during vacation and I haven't quite gotten back up to speed. Reviewing now.

          Show
          wmwood-vasey Michael Wood-Vasey added a comment - Thanks for the ping, Jim Bosch This came to me during vacation and I haven't quite gotten back up to speed. Reviewing now.
          Hide
          wmwood-vasey Michael Wood-Vasey added a comment -

          LGTM

          The matplotlib stuff will actually be fine. All of the common backends, whether interactive or not, support saving out to PDF, PNG, JPG, SVG, PS, EPS.

          Show
          wmwood-vasey Michael Wood-Vasey added a comment - LGTM The matplotlib stuff will actually be fine. All of the common backends, whether interactive or not, support saving out to PDF, PNG, JPG, SVG, PS, EPS.
          Hide
          jbosch Jim Bosch added a comment -

          Merged to master.

          Show
          jbosch Jim Bosch added a comment - Merged to master.

            People

            • Assignee:
              jbosch Jim Bosch
              Reporter:
              mfisherlevine Merlin Fisher-Levine
              Reviewers:
              Lauren MacArthur, Merlin Fisher-Levine, Michael Wood-Vasey
              Watchers:
              Jim Bosch, John Parejko, Lauren MacArthur, Merlin Fisher-Levine, Michael Wood-Vasey
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: