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

Remove obs dependency from ap_verify_testdata and ap_pipe_testdata

    XMLWordPrintable

    Details

    • Type: Improvement
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: ap_pipe, ap_verify
    • Labels:
      None
    • Story Points:
      4
    • Sprint:
      AP S21-1 (December)
    • Team:
      Alert Production

      Description

      The ap_verify dataset framework was originally designed on the principle that ap_verify itself should not depend on any obs packages or on any datasets, and that users should set up only those that they need. Datasets have a dependency on the obs package they need, so that the package is (almost) guaranteed to be available if the dataset is set up. As far as I know, the current CI system assumes this behavior.

      However, this causes a problem with ap_verify_testdata, because the package is included in some Stack builds despite being only an optional dependency of ap_verify. This causes frequent downloads of ap_verify_testdata, which would be prevented if ap_verify_testdata had no dependencies and it was ap_verify that guaranteed the obs package. This change would make ap_verify_testdata unusable for general processing, but this is already the case because ap_verify_testdata is not registered with ap_verify as a known dataset (but we would no longer be able to remove this special-casing in the future).

      ap_pipe_testdata also has a dependency (on obs_decam), but is a conventional test data repository rather than an ap_verify dataset. Its dependencies should also be reworked as part of this ticket.

      I think we can keep ap_pipe and ap_verify from having hard dependencies on obs packages if we make them optional. However, this requires reworking the test code that checks if testdata are available; see jointcal for one way to do this.

      Removing the obs_lsst dependency will also break ap_verify_testdata's Gen 3 maintenance script. However, as long as it's obvious to the script caller that the problem is a missing package, I don't think anything needs to be done.

        Attachments

          Issue Links

            Activity

            krzys Krzysztof Findeisen created issue -
            krzys Krzysztof Findeisen made changes -
            Field Original Value New Value
            Epic Link DM-25139 [ 435257 ]
            krzys Krzysztof Findeisen made changes -
            Description The {{ap_verify}} dataset framework was originally designed on the principle that {{ap_verify}} itself should not depend on any obs packages or on any datasets, and that users should set up only those that they need. Datasets have a dependency on the {{obs}} package they need, so that the package is (almost) guaranteed to be available if the dataset is set up. As far as I know, the current CI system assumes this behavior.

            However, this causes a problem with {{ap_verify_testdata}}, because the package is included in some Stack builds despite being only an optional dependency of {{ap_verify}}. This causes frequent downloads of {{ap_verify_testdata}}, which would be prevented if {{ap_verify_testdata}} had no dependencies and it was {{ap_verify}} that guaranteed the obs package. This change would make {{ap_verify_testdata}} unusable for general processing, but this is already the case because {{ap_verify_testdata}} is not registered with {{ap_verify}} as a known dataset (buy we would no longer be able to remove this special-casing in the future).

            {{ap_pipe_testdata}} also has a dependency (on {{obs_decam}}), but is a conventional test data repository rather than an {{ap_verify}} dataset. Its dependencies should also be reworked as part of this ticket.

            I think we can keep {{ap_pipe}} and {{ap_verify}} from having hard dependencies on obs packages if we make them optional. However, this requires reworking the test code that checks if testdata are available; see [jointcal|https://github.com/lsst/jointcal/blob/2ce9f257647894f085e5c18e5f0136ea5263cc74/tests/test_jointcal_decam.py#L49-L57] for one way to do this.

            Removing the {{obs_lsst}} dependency will also break {{ap_verify_testdata}}'s Gen 3 maintenance script. However, as long as it's obvious to the script caller that the problem is a missing package, I don't think anything needs to be done.
            The {{ap_verify}} dataset framework was originally designed on the principle that {{ap_verify}} itself should not depend on any obs packages or on any datasets, and that users should set up only those that they need. Datasets have a dependency on the {{obs}} package they need, so that the package is (almost) guaranteed to be available if the dataset is set up. As far as I know, the current CI system assumes this behavior.

            However, this causes a problem with {{ap_verify_testdata}}, because the package is included in some Stack builds despite being only an optional dependency of {{ap_verify}}. This causes frequent downloads of {{ap_verify_testdata}}, which would be prevented if {{ap_verify_testdata}} had no dependencies and it was {{ap_verify}} that guaranteed the obs package. This change would make {{ap_verify_testdata}} unusable for general processing, but this is already the case because {{ap_verify_testdata}} is not registered with {{ap_verify}} as a known dataset (but we would no longer be able to remove this special-casing in the future).

            {{ap_pipe_testdata}} also has a dependency (on {{obs_decam}}), but is a conventional test data repository rather than an {{ap_verify}} dataset. Its dependencies should also be reworked as part of this ticket.

            I think we can keep {{ap_pipe}} and {{ap_verify}} from having hard dependencies on obs packages if we make them optional. However, this requires reworking the test code that checks if testdata are available; see [jointcal|https://github.com/lsst/jointcal/blob/2ce9f257647894f085e5c18e5f0136ea5263cc74/tests/test_jointcal_decam.py#L49-L57] for one way to do this.

            Removing the {{obs_lsst}} dependency will also break {{ap_verify_testdata}}'s Gen 3 maintenance script. However, as long as it's obvious to the script caller that the problem is a missing package, I don't think anything needs to be done.
            krzys Krzysztof Findeisen made changes -
            Link This issue relates to DM-16773 [ DM-16773 ]
            swinbank John Swinbank made changes -
            Rank Ranked higher
            swinbank John Swinbank made changes -
            Epic Link DM-25139 [ 435257 ] DM-26810 [ 439762 ]
            krzys Krzysztof Findeisen made changes -
            Sprint AP F20-6 (November) [ 1052 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked lower
            krzys Krzysztof Findeisen made changes -
            Sprint AP F20-6 (November) [ 1052 ] AP F20-7 (December) [ 1059 ]
            krzys Krzysztof Findeisen made changes -
            Rank Ranked higher
            krzys Krzysztof Findeisen made changes -
            Status To Do [ 10001 ] In Progress [ 3 ]
            krzys Krzysztof Findeisen made changes -
            Reviewers John Parejko [ parejkoj ]
            Status In Progress [ 3 ] In Review [ 10004 ]
            krzys Krzysztof Findeisen made changes -
            Link This issue relates to DM-27857 [ DM-27857 ]
            Parejkoj John Parejko made changes -
            Status In Review [ 10004 ] Reviewed [ 10101 ]
            krzys Krzysztof Findeisen made changes -
            Resolution Done [ 10000 ]
            Status Reviewed [ 10101 ] Done [ 10002 ]

              People

              Assignee:
              krzys Krzysztof Findeisen
              Reporter:
              krzys Krzysztof Findeisen
              Reviewers:
              John Parejko
              Watchers:
              John Parejko, Krzysztof Findeisen, Meredith Rawls, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins

                  No builds found.