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

Migrate obs packages to python logging where appropriate

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Team:
      Architecture
    • Urgent?:
      No

      Description

      Now that butler and Task use python logging by default, remove lsst.log usage from obs packages (except for gen2 mappers).

        Attachments

          Activity

          Hide
          tjenness Tim Jenness added a comment -

          Meredith Rawls would you be able to do this review? It's four obs packages but the changes are few and trivial.

          Show
          tjenness Tim Jenness added a comment - Meredith Rawls would you be able to do this review? It's four obs packages but the changes are few and trivial.
          Hide
          mrawls Meredith Rawls added a comment -

          I am happy to add this to my queue, but it's behind two other reviews I've been neglecting, and will likely happen next week.

          Show
          mrawls Meredith Rawls added a comment - I am happy to add this to my queue, but it's behind two other reviews I've been neglecting, and will likely happen next week.
          Hide
          tjenness Tim Jenness added a comment -

          Thanks. Not urgent – it's a small cleanup rather than important functionality.

          Show
          tjenness Tim Jenness added a comment - Thanks. Not urgent – it's a small cleanup rather than important functionality.
          Hide
          lauren Lauren MacArthur added a comment -

          I had a quick look and approved the PRs (with some minor comments in one).  I did notice a few “clean-ups” you may be ignoring ‘cuz gen2 (e.g.
          [obs]Mappers
          https://github.com/lsst/obs_decam/blob/master/python/lsst/obs/decam/decamNullIsr.py#L72
          https://github.com/lsst/obs_decam/blob/master/python/lsst/obs/decam/ingestCalibs.py#L98
          https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsstCam.py#L99-L100),
          which seems a-ok to me (but do have a look at that last one)! Happy to hit Review Complete (I already see the happy Jenkins!) if you’d like to reassign to me (but no offence whatsoever if you’d prefer to wait for a perusal from Meredith’s keen eyes!)

          Show
          lauren Lauren MacArthur added a comment - I had a quick look and approved the PRs (with some minor comments in one).  I did notice a few “clean-ups” you may be ignoring ‘cuz gen2 (e.g. [obs] Mappers https://github.com/lsst/obs_decam/blob/master/python/lsst/obs/decam/decamNullIsr.py#L72 https://github.com/lsst/obs_decam/blob/master/python/lsst/obs/decam/ingestCalibs.py#L98 https://github.com/lsst/obs_lsst/blob/master/python/lsst/obs/lsst/translators/lsstCam.py#L99-L100 ), which seems a-ok to me (but do have a look at that last one)! Happy to hit Review Complete (I already see the happy Jenkins!) if you’d like to reassign to me (but no offence whatsoever if you’d prefer to wait for a perusal from Meredith’s keen eyes!)
          Hide
          tjenness Tim Jenness added a comment -

          Thanks. I've reassigned the ticket.

          I was deliberately not touching the gen2 mappers – daf_persistence isn't using python logging and I have no intention of changing it. I will take a look at the obs_lsst one though.

          Show
          tjenness Tim Jenness added a comment - Thanks. I've reassigned the ticket. I was deliberately not touching the gen2 mappers – daf_persistence isn't using python logging and I have no intention of changing it. I will take a look at the obs_lsst one though.
          Hide
          lauren Lauren MacArthur added a comment -

          LGTM

          Show
          lauren Lauren MacArthur added a comment - LGTM
          Hide
          mrawls Meredith Rawls added a comment -

          Lauren gets a gold star for fastest review thank you!

          Show
          mrawls Meredith Rawls added a comment - Lauren gets a gold star for fastest review thank you!

            People

            Assignee:
            tjenness Tim Jenness
            Reporter:
            tjenness Tim Jenness
            Reviewers:
            Lauren MacArthur
            Watchers:
            Lauren MacArthur, Meredith Rawls, Tim Jenness
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Dates

              Created:
              Updated:
              Resolved:

                Jenkins

                No builds found.