tweak/enhance auto data install

[mperrin]

Some enhancements building on the very nice data installation function added in spacetelescope/webbpsf#932 by @WilliamJamieson :

• If the data was already installed into the default data path by some prior call to auto_download_stpsf_data, then there's no need to emit any warning, just use that automatically installed data.
• Add a back compatibility function get_webbpsf_data_dir which is a deprecated equivalent for get_stpsf_data_dir. This is simply to allow any existing webbpsf user code that calls webbpsf.utils.get_webbpsf_data_dir() to continue to work without error. The deprecation is marked using the recently-added standard library @deprecated decorator, or the back ported equivalent depending on Python version.
• Update the installation docs to include the information about the automatic data install, and no longer say that it's required to manually install the data.

[mperrin]

hmm, a side issue in this: I was hoping to use the recently-added standard library @deprecated decorator, but that's not yet in all the versions of Python we support. And the back port of that decorator in library typing_extensions isn't installed by default either. We could add that to the requirements but that feels like overkill.

Mostly I would like to avoid unnecessary effort writing our own deprecation warning machinery -- this very much feels like something we should be able to get from upstream somewhere.

Let me think about an alternative way to do that...

[mperrin]

OK, I think it will work to, in webbpsf.__init__.py, after doing the sys.modules magic, then we can do

webbpsf.utils.get_webbpsf_data_path = stpsf.utils.get_stpsf_data_path

and I think that works, based on some testing here. And, no particular deprecation warning is needed in this case, because it'll be covered under the overall deprecation warning at the top of webbpsf.

[BradleySappington]

OK, I think it will work to, in webbpsf.__init__.py, after doing the sys.modules magic, then we can do

webbpsf.utils.get_webbpsf_data_path = stpsf.utils.get_stpsf_data_path

and I think that works, based on some testing here. And, no particular deprecation warning is needed in this case, because it'll be covered under the overall deprecation warning at the top of webbpsf.

get_webbpsf_data_path will not actually be used once webbpsf2.0.0 is released. Webbpsf will directly call get_stpsf_data_path.

[BradleySappington]

installation.rst changes are great
Please undo the utils.py changes for reasons in the comment

[BradleySappington]

Good addition! Thanks for cleaning up the docs!

[BradleySappington]

This change isn't necessary as this logic already exists within auto_download_stpsf_data (which also sets the STPSF_PATH which is necessary)

auto_download_stpsf_data() will make a new default directory (only if one doesn't exist), and download the data (only if the directory is empty).

[mperrin]

My point here was intentionally refactoring to allow checking for a prior successful call to auto_download_stpsf_data, prior to displaying the big giant warning message about trying to download the data. If the data has already been downloaded successfully, it’s incorrect to display a warning that says “stpsf will try to download the data”.

The current code will display the warning text every time the package is imported in a new Python session, even if the data is already indeed present and fine in the default directory. That’s excess warnings for no benefit to the user.

[BradleySappington]

I see your concern (deleted my previous reply because I misunderstood what download message you were referring to)

I think its important that the Environment variable $STPSF_PATH is not set! warning is still displayed.

May just make more sense to move the MISSING_STPSF_DATA_MESSAGE inside auto_download_stpsf_data inside the if not any(default_path.iterdir()): codeblock.

[mperrin]

get_webbpsf_data_path will not actually be used once webbpsf2.0.0 is released. Webbpsf will directly call get_stpsf_data_path.

My point is about existing user code that calls that function directly. That function is currently part of the public API of webbpsf, and I know for a fact there are existing notebooks and downstream scripts that call it directly. Not a lot but nonzero. I completely agree that all the internal code you updated to call get_stpsf_data_path. But again I’m trying to make sure we don’t break some existing notebooks or script that calls get_webbpsf_data_path directly.

[BradleySappington]

get_webbpsf_data_path will not actually be used once webbpsf2.0.0 is released. Webbpsf will directly call get_stpsf_data_path.

My point is about existing user code that calls that function directly. That function is currently part of the public API of webbpsf, and I know for a fact there are existing notebooks and downstream scripts that call it directly. Not a lot but nonzero. I completely agree that all the internal code you updated to call get_stpsf_data_path. But again I’m trying to make sure we don’t break some existing notebooks or script that calls get_webbpsf_data_path directly.

AH! Yes thats a good call. Overlooked that we should verify there are no other public API calls that specifically have "webbpsf" as part of the naming convention.

(just confirmed that its only auto_download_webbpsf_data and get_webbpsf_data_path)