Details
-
Type:
Improvement
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:None
-
Story Points:4
-
Epic Link:
-
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
Field | Original Value | New Value |
---|---|---|
Epic Link |
|
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. |
Rank | Ranked higher |
Sprint | AP F20-6 (November) [ 1052 ] |
Rank | Ranked lower |
Sprint | AP F20-6 (November) [ 1052 ] | AP F20-7 (December) [ 1059 ] |
Rank | Ranked higher |
Status | To Do [ 10001 ] | In Progress [ 3 ] |
Reviewers | John Parejko [ parejkoj ] | |
Status | In Progress [ 3 ] | In Review [ 10004 ] |
Status | In Review [ 10004 ] | Reviewed [ 10101 ] |
Resolution | Done [ 10000 ] | |
Status | Reviewed [ 10101 ] | Done [ 10002 ] |