Cache star coordinates

[jeremyneveu]

Avoid multiple queries to Simbad

Coordinates are stored as pickle files in .cache/astroquery folder in the extractor subpackage

[coveralls]

Pull Request Test Coverage Report for Build 14511196164

Details

• 28 of 39 (71.79%) changed or added relevant lines in 1 file are covered.
• 7 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.05%) to 71.365%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
spectractor/extractor/targets.py	28	39	71.79%

Files with Coverage Reduction	New Missed Lines	%
spectractor/extractor/targets.py	7	55.11%

Totals
Change from base Build 13558643118:	-0.05%
Covered Lines:	5762
Relevant Lines:	8074

---

💛 - Coveralls

[jeremyneveu]

@timj @erykoff are you happy with this proposition to cache star coordinates and skip simbad queries?

[timj]

The cache directory location can't work as written.

I think you need to have a combination of a read-only resource inside the python package that is accessed via importlib.resources.

ie do a look up first in the package, then look in the astropy cache and then do the simbad query itself and write to the astropy cache.

[timj]

If you don't want to special case this one star in the source code itself, my other suggestion was to have the test setUp write the ecsv file to the astropy cache so that when the test runs it reads from the cache.

[jeremyneveu]

I put a decorator for the test, like

@astropy.config.set_temp_cache(os.path.join(os.path.abspath(os.path.dirname(__file__)), "data", "cache"))

Is it sufficient? Otherwise can you be more explicit concerning your idea "to have the test setUp write the ecsv file to the astropy cache so that when the test runs it reads from the cache." ?

[timj]

I think your decorator idea should be okay so long as nothing writes to that directory. My idea was a more explicit setUp method (or pytest equivalent) that copied the file from the test dir to the cache directory so that the test would pick it up from the cache -- that doesn't require writes to the tests/ directory but if the cache is only being accessed for that test it should be fine.

[jeremyneveu]

OK, I hope I understood well your recommandation. I have just pushed a modification of setup.py

[timj]

I don't see any changes to a test for setting the cache dir so that the ecsv file is read.

[jeremyneveu]

I don't see any changes to a test for setting the cache dir so that the ecsv file is read.

Sorry the git push failed. Now it's online.

[timj]

I'm sorry. When I said "copy the file to the cache in setup" I meant in the test code just before the test runs. Not having setup.py copy the file -- For tests with unittest there is a setUp method that is called that can do things that the tests need. I'm not sure what the equivalent would be for your tests since you aren't using unittest. I didn't even realize you were using the deprecated setup.py to build the package. There is no guarantee that when test code runs that that cache file written during package installation would still exist.

PS I saw recently that Astropy cache tooling does allow you to name the cache directory so you can call it "spectractor", just like astroquery calls it "astroquery".