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

Standardize Gen3 instrument class names and location

    Details

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

      Description

      Currently the gen3 instrument classes have no consistency in naming or module path. This adds to the confusion. Currently we have:

      • lsst.obs.subaru.gen3.hsc.instrument.HyperSuprimeCam
      • lsst.obs.decam.instrument.DarkEnergyCamera
      • lsst.obs.lsst.gen3.instrument.LatissInstrument
      • lsst.obs.lsst.gen3.instrument.Ts8Instrument
      • lsst.obs.lsst.gen3.instrument.Ts3Instrument
      • lsst.obs.lsst.gen3.instrument.UcdCamInstrument
      • lsst.obs.lsst.gen3.instrument.PhosimInstrument
      • lsst.obs.lsst.gen3.instrument.ImsimInstrument
      • lsst.obs.lsst.gen3.instrument.LsstComCamInstrument
      • lsst.obs.lsst.gen3.instrument.LsstCamInstrument

      corresponding to instrument names of HSC, DECam, LATISS, LSST-TS8, LSST-TS3, LSST-UCDCam, PhoSim, LSST-ImSim, LSST-ComCam, lsstCam.

      Instrument names have to be unique in Gen3 since telescope is not part of the data model. This is why TS8 is not TS8. lsstCam should probably be LSSTCam.

      This ticket will try to clean things up a bit.

      obs_cfht has no gen3 support at this time.

        Attachments

          Issue Links

            Activity

            Hide
            Parejkoj John Parejko added a comment -

            We definitely need to get rid of the `gen3` submodules.

            I'm personally torn on whether to have `Instrument` in the class name or not. Advantage: it makes it abundantly clear what the class is (it's an `Instrument`! whatever that is). Disadvantage: it makes the name longer in a way that may not actually benefit users in practice.

            Show
            Parejkoj John Parejko added a comment - We definitely need to get rid of the `gen3` submodules. I'm personally torn on whether to have `Instrument` in the class name or not. Advantage: it makes it abundantly clear what the class is (it's an `Instrument`! whatever that is). Disadvantage: it makes the name longer in a way that may not actually benefit users in practice.
            Hide
            tjenness Tim Jenness added a comment -

            The consensus on Slack naming is that the HSC and DECam names are fine but the LSST ones should drop "Instrument" but prefix "Lsst" when the name is not obvious (ie LATISS is unique on its own but TS8 is not). LsstTestStand8 was proposed but I think we decided that LsstTS8 is fine (I think the S should be upper case since it's a word boundary).

            There wasn't much discussion on the instrument names themselves, but I think I will change lsstCam to LSSTCam.

            Show
            tjenness Tim Jenness added a comment - The consensus on Slack naming is that the HSC and DECam names are fine but the LSST ones should drop "Instrument" but prefix "Lsst" when the name is not obvious (ie LATISS is unique on its own but TS8 is not). LsstTestStand8 was proposed but I think we decided that LsstTS8 is fine (I think the S should be upper case since it's a word boundary). There wasn't much discussion on the instrument names themselves, but I think I will change lsstCam to LSSTCam.
            Hide
            tjenness Tim Jenness added a comment -

            Jim Bosch lots of reorganization but no code changes.

            I've put the obs_subaru files at the python/lsst/obs/subaru/ level rather than using the python/lsst/obs/hsc/ convention.

            Show
            tjenness Tim Jenness added a comment - Jim Bosch lots of reorganization but no code changes. I've put the obs_subaru files at the python/lsst/obs/subaru/ level rather than using the python/lsst/obs/hsc/ convention.
            Hide
            tjenness Tim Jenness added a comment -

            Simon Krughoff thanks for agreeing to review.

            Show
            tjenness Tim Jenness added a comment - Simon Krughoff thanks for agreeing to review.
            Hide
            krughoff Simon Krughoff added a comment -

            Looks great.

            Show
            krughoff Simon Krughoff added a comment - Looks great.

              People

              • Assignee:
                tjenness Tim Jenness
                Reporter:
                tjenness Tim Jenness
                Reviewers:
                Simon Krughoff
                Watchers:
                Jim Bosch, John Parejko, Simon Krughoff, Tim Jenness
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: