fixed cache dir bug

[Little-Ryugu]

Fixes #451 .

Changed cache_dir to default to str(pooch.os_cache("layup")) instead of None in LayupObservatory. Stops layup_furnish_spiceypy from searching for files in the Sorcha created cache.

Review Checklist for Source Code Changes

• Does pip install still work?
• Have you written a unit test for any new functions?
• Do all the units tests run successfully?
• Does Layup run successfully on a test set of input files/databases?
• Have you used black on the files you have updated to confirm python programming style guide enforcement?

[Little-Ryugu]

Don't accept/merge just yet. @mschwamb said there was a different bug before that caused a problem similar to this in orbitfit. I'm going to double-check everything first incase I missed something.

[Little-Ryugu]

@mschwamb, the fix was in pr #225 but missed the case where cache_dir is left as None. The call for Layup_observatory in orbitfit and predict was changed from:

Layup_observatory = LayupObservatory()

To:

Layup_observatory = LayupObservatory(cache_dir=cache_dir) 

This fixed the original bug in the main code, but the unit tests still have the bug as they call Layup_observatory= LayupObservatory()

There was another fix to a similar issue #286, but the fix was related to the observatory position and didn't change the unit tests calling the Sorcha cache.

[mschwamb]

I think you might want to make "layup" a global variable at the top of the file so it's clear that this is hardcoded - easier to spot if it needs to be changed.

[mschwamb]

see other comment about changes

[mschwamb]

Can you make it all caps - so that the style is the same throughout layup see

layup/src/layup/constants.py

Line 21 in ed1579a

AU_KM = AU_M / 1000.0

[Little-Ryugu]

No problem will do.

[matthewholman]

This doesn't fix #451 on the paths that matter.

Changing the default only helps callers that omit the argument. All three production callers pass it explicitly, and their own defaults are None:

• predict.py:438, orbitfit.py:1502, orbitfit.py:1555

So layup.predict(data, obscode, times) still sends None through to layup_furnish_spiceypy, which hands it to Sorcha as ar_data_file_path=None — the search #451 reports. tests/layup/test_predict.py:480 passes cache_dir=None explicitly too.

The evidence is one line above a caller:

kernels_loc = str(pooch.os_cache("layup")) if cache_dir is None else str(cache_dir)   # orbitfit.py:1554
observatory = LayupObservatory(cache_dir=cache_dir)                                   # 1555

Also, a default argument is evaluated once at import, so this freezes the cache path at module load — awkward for tests and for #448.

Suggested instead:

def __init__(self, cache_dir=None):
    if cache_dir is None:
        cache_dir = str(pooch.os_cache("layup"))
    self.cache_dir = cache_dir
    layup_furnish_spiceypy(cache_dir)

This covers explicit None, evaluates per call, matches the idiom used elsewhere (including line 434 of this file), and fixes self.cache_dir, which feeds the Horizons state cache and is currently stored as None.

A test asserting predict(..., cache_dir=None) furnishes from the layup cache would catch the regression.

[mschwamb]

See the detailed review from Matt