Implement Real and Complex type aliases
#16753
Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
astropy/utils/typing.py
Outdated
| import numpy as np | ||
|
|
||
| # The classes from the standard library `numbers` module are not suitable for | ||
| # type checking (https://github.com/python/mypy/issues/3186). |
There was a problem hiding this comment.
Just so I can come back in the future and look at the issue status without clicking:
Ah, looks like python/mypy#3186 (comment) is more helpful.
There was a problem hiding this comment.
Thanks, that link was helpful. Maybe add
# TODO: remove these when a standard way to deal with this has developed
There was a problem hiding this comment.
This feels like something astropy shouldn't have to solve... surely, every other numerical package has the same problems! That said, if in the meantime this helps, and @nstarman likes this (and has no other ideas), that should be good enough (@pllim's link was helpful to see that this is indeed a wider problem where making up your own stuff is apparently encouraged...).
My inline comments are just about making clear that we hope to eventually remove things when a good upstream solution has been found.
astropy/utils/typing.py
Outdated
| import numpy as np | ||
|
|
||
| # The classes from the standard library `numbers` module are not suitable for | ||
| # type checking (https://github.com/python/mypy/issues/3186). |
There was a problem hiding this comment.
Thanks, that link was helpful. Maybe add
# TODO: remove these when a standard way to deal with this has developed
astropy/utils/typing.py
Outdated
| # Licensed under a 3-clause BSD style license - see LICENSE.rst | ||
|
|
||
| """ | ||
| Experimental typing support for ``astropy``, subject to change |
There was a problem hiding this comment.
To further clarify, maybe
Aliases and utilities to help support typing in ``astropy``. Note that these
are experimental and may be stop-gap measures pending better official
typing support for numerical packages. Like most of ``astropy.utils``,
they are for internal use only and are subject to change without notice.
|
One more query: would it make sense to have this in |
There was a problem hiding this comment.
LGTM! This will nicely simplify some type hints.
I agree that @mhvk's suggestion of astropy.units.typing is worthwhile to consider, but I'm happy with either placement
Also, should these be linked to / discussed in the glossary?
|
I am starting to agree that these definitions should be in These types are currently only used in the private API, so there's no need to document them thoroughly. We can consider describing them in the glossary when we start using them in the public API. |
|
OK, maybe the solution is to keep them in |
The Python standard library `numbers` module defines abstract base classes for different types of numbers, but those classes are not suitable for type checking (python/mypy#3186), so `astropy` should define these types itself.
Description
The Python standard library
numbersmodule defines abstract base classes for different types of numbers, but those classes are not suitable for type checking (at least not with mypy or Pyright), soastropyshould define these types itself.Additionally, Sphinx has had trouble figuring out how to link to
numbers.Realeven though it is part of the standard library. If we defineRealourselves then Sphinx should be able to link to that definition without problems, which would be helpful for anyone reading our documentation. However, our public API does not refer toRealsince 64eccee, so this benefit is not immediately obvious, but it might become relevant when more ofastropygets annotated.One more thing worth thinking about is whether we should define
Bool: TypeAlias = bool | np.bool_, but maybe that deserves a separate pull request.