search

desihub / speclite

Lightweight utilities for working with spectroscopic data
14 stars 19 forks source link

numpy/astropy deprecation warnings #72

Closed marcelo-alvarez closed 3 years ago

marcelo-alvarez commented 3 years ago

Fixes #71 by addressing deprecation warnings for astropy 4.3.1 and numpy 1.21.2 while maintaining compatibility with astropy 4.0.1.post1 and numpy 1.19.1 on Cori.

Installation of pytest_astropy_header in python-package.yml has been added in order for github workflow unit tests to function properly.

Please review and merge / comment as appropriate.

dkirkby commented 3 years ago

This looks good, thanks. Do you have any suggestions for the failing doc tests @marcelo-alvarez?

/home/runner/work/speclite/speclite/docs/api.rst:77: WARNING: autosummary:
stub file not found 'speclite.filters.get_path_of_data_file'. Check your autosummary_generate setting.
marcelo-alvarez commented 3 years ago

This looks good, thanks. Do you have any suggestions for the failing doc tests @marcelo-alvarez?

/home/runner/work/speclite/speclite/docs/api.rst:77: WARNING: autosummary:
stub file not found 'speclite.filters.get_path_of_data_file'. Check your autosummary_generate setting.

I took a look but did not find anything to suggest.

weaverba137 commented 3 years ago

In other packages we have fixed recent documentation test failures by pinning docutils<0.18.

marcelo-alvarez commented 3 years ago

In other packages we have fixed recent documentation test failures by pinning docutils<0.18.

Thanks for this, I will look into that.

marcelo-alvarez commented 3 years ago

This looks good, thanks. Do you have any suggestions for the failing doc tests @marcelo-alvarez?

/home/runner/work/speclite/speclite/docs/api.rst:77: WARNING: autosummary:
stub file not found 'speclite.filters.get_path_of_data_file'. Check your autosummary_generate setting.

@dkirkby please check my latest commits -- I added a line in 

docs/api.rst

to skip generating any documentation for 

speclite.filters.get_path_of_data_file

since it doesn't have any documentation associated with it. I also pinned docutils < 0.18. All checks now pass.

If it makes sense, you might consider adding documentation for speclite.filters.get_path_of_data_file separately from this PR, since this one is just to remove the numpy/astropy deprecation warnings.

Please review and merge if appropriate. Thanks.

sbailey commented 3 years ago

Minor, but let's actually add a docstring for get_path_of_data_file instead of configuring the docs to skip it. speclite.filters.get_path_of_data_file traces back to speclite.utils.package_data. It could be as simple as "convenience wrapper to return location of data file" (and similar for get_path_of_data_dir). If I'm misunderstanding and the issue is deeper than just a missing docstring, let's still try to address it by fixing the docs instead of excluding the docs.

marcelo-alvarez commented 3 years ago

Minor, but let's actually add a docstring for get_path_of_data_file instead of configuring the docs to skip it. speclite.filters.get_path_of_data_file traces back to speclite.utils.package_data. It could be as simple as "convenience wrapper to return location of data file" (and similar for get_path_of_data_dir). If I'm misunderstanding and the issue is deeper than just a missing docstring, let's still try to address it by fixing the docs instead of excluding the docs.

I added docstrings and removed the exclusion from api.rst, but the doc test fails in this case, and I have been unable to find a workaround. I have fallen back to removing it from the api.rst and leaving the docstrings in.

I've merged after obtaining approval from @sbailey.