Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-47

Remove unused utils functions and change eups::productDir to getPackageDir

    XMLWordPrintable

Details

    • RFC
    • Status: Implemented
    • Resolution: Done
    • DM
    • None
    • This ticket, or the Data Management chat room

    Description

      As part of DM-2635 I propose to make the following changes to the C++ API of the "utils" package:

      Remove two functions we are not using anywhere:

      • guessSvnVersion: this is clearly useless now that we use git
      • stringToAny: we aren't using it, and it has no documentation

      Rename lsst::utils::eups::productDir to lsst::utils::getPackageDir and remove the version argument. Note that the version argument has only one valid value, so it's never been useful. This would be the implementation of DM-2635.

      Attachments

        Issue Links

          Activity

            A better solution would be to remove the use of productDir from the examples (passing the filename on the command line?), but it probably isn't worth doing as part of this issue.

            The tests are similar:

            tests/background.cc:            afwdata_dir = lsst::utils::eups::productDir("afwdata");
            tests/convolveGPU.cc:            string dataDir = lsst::utils::eups::productDir("afwdata");
            tests/statistics.cc:            afwdata_dir = lsst::utils::eups::productDir("afwdata");
            tests/testWarpGpu.cc:            string dataDir = lsst::utils::eups::productDir("afwdata");

            and we should probably remove some (do we really need C++ tests for things we test in python?) and convert the others to accept filenames from argv[], also as another issue.

            rhl Robert Lupton added a comment - A better solution would be to remove the use of productDir from the examples (passing the filename on the command line?), but it probably isn't worth doing as part of this issue. The tests are similar: tests/background.cc: afwdata_dir = lsst::utils::eups::productDir("afwdata"); tests/convolveGPU.cc: string dataDir = lsst::utils::eups::productDir("afwdata"); tests/statistics.cc: afwdata_dir = lsst::utils::eups::productDir("afwdata"); tests/testWarpGpu.cc: string dataDir = lsst::utils::eups::productDir("afwdata"); and we should probably remove some (do we really need C++ tests for things we test in python?) and convert the others to accept filenames from argv[], also as another issue.
            mjuric Mario Juric added a comment -

            rowen, thanks, I didn't realize that. Your solution seems fine, though I think it further illustrates the need to be able to easily call into Python from the C++ layer (even as a temporary solution until we get to doing something like rhl is proposing).

            Bottom line, +1 from me!

            mjuric Mario Juric added a comment - rowen , thanks, I didn't realize that. Your solution seems fine, though I think it further illustrates the need to be able to easily call into Python from the C++ layer (even as a temporary solution until we get to doing something like rhl is proposing). Bottom line, +1 from me!

            I filed DM-2641 to handle the issue brought up by Robert Lupton.

            rowen Russell Owen added a comment - I filed DM-2641 to handle the issue brought up by Robert Lupton.

            The discussion seems to have converged with no serious objections.

            rowen Russell Owen added a comment - The discussion seems to have converged with no serious objections.
            tjenness Tim Jenness added a comment -

            This is a good example of even more code being moved to the C++ layer (especially in this case because it seems that if it wasn't for these tests the code could have stayed in Python land).

            tjenness Tim Jenness added a comment - This is a good example of even more code being moved to the C++ layer (especially in this case because it seems that if it wasn't for these tests the code could have stayed in Python land).

            People

              rowen Russell Owen
              rowen Russell Owen
              Kian-Tat Lim, Mario Juric, Robert Lupton, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                Jenkins

                  No builds found.