Fix Version/s: None
Component/s: meas_astrom, pipe_tasks
DM-2636 we modified the tests to be skipped if EUPS is not available. I've had a closer look and all the ones I have glanced at seem to be easily fixable to run without EUPS. The tests seem to be using EUPS to locate the meas_astrom (effectively asking EUPS for the location of the test file), then a path to the astrometry.net test data within the tests/ directory is located and then EUPS is asked to setup astrometry_net_data using that path. Since the table files are all empty this is the equivalent to simply assigning the ASTROMETRY_NET_DATA_DIR environment variable directly to the path in the tests sub-directory.
Making this change to one of the tests seems to work so I will change the rest.
This is now done. Paul Price would you mind reviewing it? Should only take a couple of minutes. The changes are essentially the same in each of the modified test files.
- Might it be possible to factor the common code into a function?
- Picky: in the ValueError, (datapath) is the same as datapath — suggest either (datapath,) or datapath?
Thanks for the review.
Regarding a function. It's specific to these tests so not suitable for lsst.utils (although I've just noticed I need to make the same fixes to pipe_tasks). It would have to take two arguments: the location of the test and the astrometry net data directory. Are there any examples of tests in other places that import test support code?
I'll fix the datapath issue. That's a holdover from there being two items in there originally (and I usually prefer format but was keeping it as is).
See afw/tests/spatialCell.py for an example of importing another module from the tests dir. I think it's just a matter of import foo to load tests/foo.py.
Thanks. I didn't know if there was a convention so that people woudn't confuse tests/foo.py for an actual test file.
I think it does get confused (unless it's in the SConscript as something to ignore), but it doesn't hurt to run it.
I've made the suggested changes and force updated the branch. The code looks much better now and I now test the new helper function in its own test file. Please review again just to make sure I didn't misunderstand.
Now I'm pondering pipe_tasks. Two tests in there could do with the same helper code so I guess I just copy it across from meas_astrom. Seems non-optimal but it also doesn't seem like code that is generic enough to go into utils.
It also strikes me as odd that one of the pipe_tasks examples actually burrows into the test directory of meas_astrom looking for photocal astrometry.net data and test data files. I may just ignore that example for now and let it rely on eups for a bit longer.
Paul, at risk of winding you up, I've made the same changes to pipe_tasks as I made to meas_astrom. I'm using the same ticket as the fixes are identical.
My main concern is over the copying of the test helper code from meas_astrom.
I have removed eups from the example code as well, although that does involve some code duplication as the examples do not have access to the test helper code.
I think if you discover any more places where it's necessary to use setupAstrometryNetDataDir then you should put it in utils, as I'm already starting to hold my nose against the duplication.
Merged meas_astrom and pipe_tasks to master. Can't find any other candidates for the helper function.
Some of this may be in flux on
DM-3142, so you might want to hold off until I get that merged (or branch from there?).