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

prune stale obs_subaru dependencies

    XMLWordPrintable

Details

    • Story
    • Status: Done
    • Resolution: Done
    • None
    • obs_subaru
    • None
    • 4
    • Science Pipelines DM-W16-6
    • Data Release Production

    Description

      obs_subaru has some Eups dependencies that should be removed:

      • meas_extensions_multiShapelet (removed from the LSST stack, content moved to meas_modelfit)
      • meas_multifit (renamed to meas_modelfit)
      • fitsthumb (need to cherry-pick code from HSC to replace it)
      • pyfits (investigate what we use it for; it might not be needed)
      • psycopg2 (I don't think we'll ever use this on the LSST side; we should add it back to the HSC side after we re-fork)

      Attachments

        Issue Links

          Activity

            The fitsthumb replacement was cherry-picked in DM-4323, so we've already lost that dependency.

            Agreed with Paul that it's safe to drop psycopg2.

            Pyfits is used by some of the scripts in bin.src. I've not investigated whether we need them, but this dependency costs us nothing – it's easily satisfied on the LSST side.

            Neither suprime_data nor testdata_subaru exist on LSST, and I don't think they're likely to in the immediate future.

            jbosch: I've rebased your commits against the current master and added my own to drop psycopg2, suprime_data and testdata_subaru. Are you happy with that?

            swinbank John Swinbank added a comment - The fitsthumb replacement was cherry-picked in DM-4323 , so we've already lost that dependency. Agreed with Paul that it's safe to drop psycopg2. Pyfits is used by some of the scripts in bin.src . I've not investigated whether we need them, but this dependency costs us nothing – it's easily satisfied on the LSST side. Neither suprime_data nor testdata_subaru exist on LSST, and I don't think they're likely to in the immediate future. jbosch : I've rebased your commits against the current master and added my own to drop psycopg2 , suprime_data and testdata_subaru . Are you happy with that?
            jbosch Jim Bosch added a comment -

            Dropping all of those is fine, though I'll note that there's still a lot of PostgreSQL-related code in obs_subaru, and it might make sense to remove those along with the EUPS dependency on psycopg2 - but I'm happy to defer to you and price on that question.

            jbosch Jim Bosch added a comment - Dropping all of those is fine, though I'll note that there's still a lot of PostgreSQL-related code in obs_subaru, and it might make sense to remove those along with the EUPS dependency on psycopg2 - but I'm happy to defer to you and price on that question.
            price Paul Price added a comment -

            I agree on removing the PostgreSQL-related code. swinbank, let me know if you want me to do it.

            price Paul Price added a comment - I agree on removing the PostgreSQL-related code. swinbank , let me know if you want me to do it.

            If you have time to do it now, go for it. Otherwise, it's on my todo list for a dull moment.

            swinbank John Swinbank added a comment - If you have time to do it now, go for it. Otherwise, it's on my todo list for a dull moment.

            Hey Lauren – do you have time to look at this please? I think all the changes are straightforward. Almost all of the churn is caused by stripping out psycopg2 as discussed above (the HSC folks will re-add this on their own future fork).

            Note that obs_subaru is not currently used when building lsst_apps or lsst_distrib, so it doesn't get tested by a standard Jenkins run. However, Jenkins is capable of building it directly: see e.g. build #6685.

            swinbank John Swinbank added a comment - Hey Lauren – do you have time to look at this please? I think all the changes are straightforward. Almost all of the churn is caused by stripping out psycopg2 as discussed above (the HSC folks will re-add this on their own future fork). Note that obs_subaru is not currently used when building lsst_apps or lsst_distrib , so it doesn't get tested by a standard Jenkins run. However, Jenkins is capable of building it directly: see e.g. build #6685 .

            Moving this out of review until I have chance to drop in some working test cases. Should be back in a few hours.

            swinbank John Swinbank added a comment - Moving this out of review until I have chance to drop in some working test cases. Should be back in a few hours.
            swinbank John Swinbank added a comment - - edited

            Following tjenness's comment, I reinstated the testdata_subaru package and provided some minimal tests to demonstrate the basic operation of obs_subaru. These make no claim to be comprehensive, but will ultimately be supplemented by ci_hsc (DM-3663, DM-4251).

            As well as the changes on tickets/DM-3518 in obs_subaru, please also sanity-check the new testdata_subaru package.

            Note that this currently will not cleanly run through Jenkins thanks to DM-4733, but you should be able to clone and demonstrate that the tests pass on your own system. I won't merge to master until that issue has been resolved.

            swinbank John Swinbank added a comment - - edited Following tjenness 's comment, I reinstated the testdata_subaru package and provided some minimal tests to demonstrate the basic operation of obs_subaru . These make no claim to be comprehensive, but will ultimately be supplemented by ci_hsc ( DM-3663 , DM-4251 ). As well as the changes on tickets/DM-3518 in obs_subaru , please also sanity-check the new testdata_subaru package. Note that this currently will not cleanly run through Jenkins thanks to DM-4733 , but you should be able to clone and demonstrate that the tests pass on your own system. I won't merge to master until that issue has been resolved.

            I put some comments on the pull request. I still have to pull down the branch to confirm that it compiles, but I'll wait to hear your response to the comments (nothing major, but a few items would require re-checking).

            lauren Lauren MacArthur added a comment - I put some comments on the pull request. I still have to pull down the branch to confirm that it compiles, but I'll wait to hear your response to the comments (nothing major, but a few items would require re-checking).

            Thanks for the comments – I've responded to them on the PR.

            swinbank John Swinbank added a comment - Thanks for the comments – I've responded to them on the PR.

            Following discussion on the PR, I've added two tiny additional commits to this ticket:

            Could you please also consider them in the review?

            swinbank John Swinbank added a comment - Following discussion on the PR, I've added two tiny additional commits to this ticket: https://github.com/lsst/obs_subaru/commit/2067d129cf45eebd5555425d094f66f4202a64e1 https://github.com/lsst/obs_subaru/commit/5d5d421ee24f49149bc343201cf72189c40450a6 Could you please also consider them in the review?

            All looks good. Feel free to merge to master (when the Jenkins integration is taken care of).

            lauren Lauren MacArthur added a comment - All looks good. Feel free to merge to master (when the Jenkins integration is taken care of).

            Added a further commit which addresses the pyfits API change per DM-5007. It makes sense to include that on this ticket since:

            • Without the fix, the new test cases here won't run;
            • The test cases here demonstrate that the fix actually works!
            swinbank John Swinbank added a comment - Added a further commit which addresses the pyfits API change per DM-5007 . It makes sense to include that on this ticket since: Without the fix, the new test cases here won't run; The test cases here demonstrate that the fix actually works!

            People

              swinbank John Swinbank
              jbosch Jim Bosch
              Lauren MacArthur
              Jim Bosch, John Swinbank, Lauren MacArthur, Paul Price
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Jenkins

                  No builds found.