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
Tidy up of the io doc strings and API page. Deprecate toplevel io namespace. #7537
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand how some of these tests aren't throwing an ImportError
but sure.
sunpy/io/tests/test_filetools.py
Outdated
assert mock.call_args[0][0] == url | ||
assert isinstance(hdulist, list) | ||
assert len(hdulist) == 1 | ||
assert len(hdulist[0]) == 2 | ||
assert isinstance(hdulist[0][0], np.ndarray) | ||
assert isinstance(hdulist[0][1], sunpy.io.header.FileHeader) | ||
assert isinstance(hdulist[0][1], sunpy.io._header.FileHeader) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this fail since you've removed import sunpy.io
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still import it no?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does that still work?!
It surely won't get it from import sunpy
so?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imported it for my sanity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm an idiot. Nevermind. Looks good.
sunpy/io/__init__.py
Outdated
warn_deprecated('The toplevel space of the io subpackage was never intended for public use and will be removed in the future.') | ||
from sunpy.io._file_tools import read_file, write_file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is wrong.
I get the message if I do:
In [3]: import sunpy.io._header
WARNING: SunpyDeprecationWarning: The toplevel space of the io subpackage was never intended for public use and will be removed in the future. [sunpy.io]
I should not be raising this warning if someone does import that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or even:
import sunpy.io.special.asdf
As it stands, we can't merge this till I fix this.
Co-authored-by: Will Barnes <will.t.barnes@gmail.com>
TODO: Fix CI |
Changelog check will fail as I had to change other changelogs but I did add one.