Fix Version/s: None
Sprint:Science Pipelines DM-S15-2
RFC-44 we want a simple function in utils that returns the path to a package given a package name. This has the same API as eups.getProductDir, but hides our dependence on eups, as per the RFC.
DM-2774 Remove eups dependency from tests
DM-2636 Update code to use the function provided in DM-2635
- is blocked by
RFC-47 Remove unused utils functions and change eups::productDir to getPackageDir
|Sprint||Science Pipelines DM-S15-2 [ 151 ]|
|Reviewers||Tim Jenness [ tjenness ]|
|Status||To Do [ 10001 ]||In Review [ 10004 ]|
|Summary||Provide a function to return the path to a package, given it name||Provide a function to return the path to a package, given its name|
After putting it up for review I realized there was a C++ wrapper as well. So I modified the code as follows:
- Rename the C++ wrapper from lsst::utils::eups::productDir to lsst::utils::getPackageDir and removed the extra version argument that was not actually usable. This function throws an exception if the package is not found.
- Made the Python code a SWIG wrapper of the C++ code. So now getPackageDir does throw an exception, which (having converted all the code in afw) I think is a marked improvement.
My apologies for the post-review-state changes. However, I think the results are significantly improved, and I've implemented all suggested changes where they still make sense.
Looks okay. How do the AFW patches relate to
DM-2636? Do all the changes have to happen at once because the previous EUPS utility function has disappeared?
|Status||In Review [ 10004 ]||Reviewed [ 10101 ]|
I changed afw on the same ticket because some C++ examples and tests were using lsst::utils::eups::packageDir, which has been replaced by lsst::utils::getPackageDir. While I was at it, I also updated the python code that was using eups.packageDir, though I could have waited until
DM-2636 for that. afw is the only package I found that uses lsst::utils::eups::packageDir so I think this ticket is nicely self-contained. I will start a buildbot run shortly to see if I missed anything.
That's fine. I was a bit worried that splitting this task into 2 tickets would only work if the old interface remained until the second ticket was completed.
|Resolution||Done [ 10000 ]|
|Status||Reviewed [ 10101 ]||Done [ 10002 ]|
|Team||Alert Production [ 10300 ]|
I have just noticed that diffimLib.py in ip_diffim (derived from diffimLib.i attempts to use guessSvnVersion in diffimlib.version. The entire version function seems anomalous. In fact, I'm suspicious of any code that is trying to deal with $HeadURL$ (pex_logging being the other package if you assume that sconsUtils has to have support for Svn).
Yup, that's a relic, and it's safe to just delete it. We now machine-generate a version.py as part of every sconsUtils-based build.
|Assignee||Russell Owen [ rowen ]||Joshua Hoblitt [ jhoblitt ]|
|Assignee||Joshua Hoblitt [ jhoblitt ]||Russell Owen [ rowen ]|
I added getPackageDir to utils on tickets/
DM-2635and included a simple unit test.
I chose to implement the function using environment variables, rather than using the productDir method that sconsUtils adds to eups. If standard eups included productDir as a method, or the function was clearly part of sconsUtils instead of having sconsUtils add it to eups, I would have chosen differently, because it makes me nervous to have two different implementations. But at least the implementation is trivial and the sconsUtils version will only be used inside of sconsUtils once