Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a config option to disable caching? #1511

Closed
adrn opened this issue Jul 24, 2019 · 11 comments
Closed

Add a config option to disable caching? #1511

adrn opened this issue Jul 24, 2019 · 11 comments

Comments

@adrn
Copy link
Member

adrn commented Jul 24, 2019

Via email, @dnidever asked me if there was a way to disable caching with astroquery. It looks like each .query*() method has a cache=False option, but is there a way to do this globally? It would be nice to add a config option to handle this, if it doesn't already exist.

@dnidever
Copy link

Thanks for opening the issue Adrian.
I'm using Vizier.query_region() and don't see a cache=False option. I also don't see any Vizier.cache attribute only cache_location.

@keflavich
Copy link
Contributor

Right, <astroquery module>.cache_location is the directory it writes to; a hacky workaround is to do Vizier.cache_location = '/dev/null'. There should be cache=T/F options for every module....

@keflavich
Copy link
Contributor

query_region accepts the cache=<bool> option, but it's not documented properly.

@eteq
Copy link
Member

eteq commented Jul 25, 2019

@keflavich - isn't it also possible to have cache_location be None? I know there's at least some code that checks for that (although I'm not sure how uniformly implemented it is...)

@eteq
Copy link
Member

eteq commented Jul 25, 2019

(If so, I suppose it should be documented clearly, though!)

@keflavich
Copy link
Contributor

@dnidever
Copy link

Thanks. I'll try out the cache_location=None option. But that seems like a non-intuitive way of turning off caching to me.

@keflavich
Copy link
Contributor

Can you suggest what would be most intuitive? I agree that it's not obvious, since it's not documented at all, but that would probably have been my first suggested approach once we do document it.

@dnidever
Copy link

dnidever commented Jul 26, 2019 via email

@keflavich
Copy link
Contributor

OK, that sounds good - cache=True/False exists and is mostly documented, but some documents haven't propagated (because of the way _async vs sync functions are defined).

I'm torn about defaulting caching to False, since that can make a lot of potentially cheap operations very expensive. But, I think caching in its present form isn't well enough communicated, as you point out. Some options we should consider:

(1) Warn the first time caching is done in a session so users know that caching happens
(2) Cache to tempfiles by default so that they automatically clear at the end of session. This prevents caching between sessions, but maybe that's just a generally better approach anyway?

Any other ideas? Neither of these is difficult to implement, and I think they're both reasonable.

@bsipocz
Copy link
Member

bsipocz commented Nov 10, 2022

#1634 meant to address this, reopen if it's not the case.

@bsipocz bsipocz closed this as completed Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants