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

Rename lsst.verify.compatibility to gen2compatibility

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: verify
    • Labels:
      None

      Description

      The compatibility package was originally proposed in DMTN-098 as a subpackage of a lsst.verify.measurements package that would contain extra infrastructure. However, following RFC-550, this package was instead added to lsst.verify. Since lsst.verify is much broader in scope than the proposed lsst.verify.measurements would have been, the name compatibility has caused much confusion in the new context (what needs to be compatible with what?).

      We can alleviate some of the confusion by renaming compatibility to gen2compatibility. This must be done before the MetricTask framework is widely adopted.

        Attachments

          Issue Links

            Activity

            Hide
            krzys Krzysztof Findeisen added a comment -

            Hi Jonathan Sick, since this is a structural change to `lsst.verify`, can you take a look and confirm that this won't cause any non-obvious trouble?

            Show
            krzys Krzysztof Findeisen added a comment - Hi Jonathan Sick , since this is a structural change to `lsst.verify`, can you take a look and confirm that this won't cause any non-obvious trouble?
            Hide
            jsick Jonathan Sick added a comment -

            If you're going to the trouble of renaming this, I'll be honest and say that the name is still obscure. Could you consider making the namespace of this compatibility layer lsst.verify.gen2tasks and then when things are working in Gen3 the namespace is `lsst.verify.tasks`?

            I think having those two parallel namespaces might make the transition easier to understand as well. And yes, as far I as understand that'd mean moving python/lsst/verify/metricTask.py to python/lsst/verify/tasks/metricTask and making APIs available from that namespace (e.g. lsst.verify.tasks.MetricComputationError).

            This has all the advantages of the verify_measurements plan you originally proposed, without the unnecessary overhead of creating a new EUPS package.

            Show
            jsick Jonathan Sick added a comment - If you're going to the trouble of renaming this, I'll be honest and say that the name is still obscure. Could you consider making the namespace of this compatibility layer lsst.verify.gen2tasks and then when things are working in Gen3 the namespace is `lsst.verify.tasks`? I think having those two parallel namespaces might make the transition easier to understand as well. And yes, as far I as understand that'd mean moving python/lsst/verify/metricTask.py to python/lsst/verify/tasks/metricTask and making APIs available from that namespace (e.g. lsst.verify.tasks.MetricComputationError). This has all the advantages of the verify_measurements plan you originally proposed, without the unnecessary overhead of creating a new EUPS package.
            Hide
            krzys Krzysztof Findeisen added a comment -

            No objections to gen2tasks.

            As for tasks, didn't you say that the code should live in the lsst.verify namespace (I thought there were technical reasons why it mattered, but I can't find mention of them now)? Or are you saying that the tasks package should not get imported into verify proper?

            Show
            krzys Krzysztof Findeisen added a comment - No objections to gen2tasks . As for tasks , didn't you say that the code should live in the lsst.verify namespace (I thought there were technical reasons why it mattered, but I can't find mention of them now)? Or are you saying that the tasks package should not get imported into verify proper?
            Hide
            jsick Jonathan Sick added a comment -

            I guess it's a judgement call, and we could go either way. If, after doing the compatibility subpackage work, you like that all the task framework stuff is in a namespace, then you could by all means have a lsst.verify.tasks namespace for the final implementation. Putting all the public APIs in lsst.verify would also be fine. I'll leave that up to you.

            I think my original concern was mostly against having an extra EUPS package. Mostly I just want the pipeline interface and the primitives to be documented and developed in the same place.

            Show
            jsick Jonathan Sick added a comment - I guess it's a judgement call, and we could go either way. If, after doing the compatibility subpackage work, you like that all the task framework stuff is in a namespace, then you could by all means have a lsst.verify.tasks namespace for the final implementation. Putting all the public APIs in lsst.verify would also be fine. I'll leave that up to you. I think my original concern was mostly against having an extra EUPS package. Mostly I just want the pipeline interface and the primitives to be documented and developed in the same place.
            Hide
            jsick Jonathan Sick added a comment -

            All that said, I do love the simplicity of something likeĀ "from lsst.verify import MetricTask" so if we can still do that, forget everything I said about lsst.verify.tasks

            Show
            jsick Jonathan Sick added a comment - All that said, I do love the simplicity of something likeĀ " from lsst.verify import MetricTask " so if we can still do that, forget everything I said about lsst.verify.tasks
            Hide
            krzys Krzysztof Findeisen added a comment - - edited

            Ok then, I've renamed compatibility to gen2tasks but am keeping the gen 3-compatible APIs in lsst.verify. Can you take another look?

            Show
            krzys Krzysztof Findeisen added a comment - - edited Ok then, I've renamed compatibility to gen2tasks but am keeping the gen 3-compatible APIs in lsst.verify . Can you take another look?
            Hide
            jsick Jonathan Sick added a comment -

            LGTM

            Show
            jsick Jonathan Sick added a comment - LGTM

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              Jonathan Sick
              Watchers:
              Jonathan Sick, Krzysztof Findeisen
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.