-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Make typing_extensions
an (optional?) astropy dependency
#10689
Comments
typing_extensions
an astropy dependencytyping_extensions
an astropy dependency
Could you clarify which (especially non-experimental) features it would actually add for newer Python versions? The project page primarily discusses 3.4-3.6, and those alone might weaken the case a bit, since the schedule for dropping 3.6 support is already under discussion. |
What I meant was that there are a number of types which are py3.7+ or py3.8+ or even py3.9+. Until our minimum version is 3.9, then not all the static type will be available. To me personally, the most useful are:
def func(x: Literal["a", "b"]):
.... means
Also useful are:
There are also PEPs for adding other types, like 603 for frozen maps, and 604 for the pipe operator to create union types. These will be implemented in the
Edit: also numpy's |
This adds maintenance burden for the core maintainers, especially given that our minversion for Python has not caught up to your needs yet. Some cost-benefit discussions are needed. |
Isn't that part of |
Beyond ensuring that a package has consistent and equivalent functionality across all supported python versions, I'm not that familiar with the cost aspect of adding dependencies. Could you highlight a couple so I can look into them and how they relate to the |
Not the dedicated static type. This one is actually more flexible since its a |
I think the only package beyond |
I've been reading the discussion around |
Numpy's next major release will have types information, and they are using |
Thanks for pointing this out! I'm looking at the code, if |
I'd vote for optional dependency. Astropy has a pretty strong policy of keeping absolutely required dependencies at the bare minimum. |
Many of us consider dynamic typing to be a feature, not a bug. 😄 |
@taldcroft Thanks for the suggestion. I've been looking at implementation with regards to #10662 , and I think this would be a workable solution. Anyone who wants type annotations can use the optional dependency. The same feature set for Quantity type hint annotation is provided by a simple compatibility class (already implemented). I'm not sure how these types of issues normally progress. Is there a voting mechanism, or what? |
typing_extensions
an astropy dependencytyping_extensions
an optional astropy dependency
It does not seem there is very strong objection to adding this package as an optional dependency, but @pllim mentioned a point early in the discussion about maintenance burden. Does that still apply for an optional dependency? We definitely want the infrastructure maintainers on board with the change and they need to approve. |
Yes, it does. For example, remember that time when |
The argument here would be that python and numpy are very much moving towards typing, and since |
I guess it will be a while before our minversion will be Python 3.8. 🤔 I am not against it, but I just want us to consider all angles before proceeding. |
@mhvk, I would be willing to trouble-shoot issues as they arise. I've taken an interest in learning static typing with regards to my own code, and would be happy to contribute that to Astropy. |
works better with astropy#10689, but even without. Signed-off-by: nstarman <nstarkman@protonmail.com>
typing_extensions
an optional astropy dependencytyping_extensions
an (optional?) astropy dependency
@embray discussion points about the
While having |
I'm revisiting #10662 and re-encountering the problems related to not having an
There a larger conversation to be had on how Astropy wants to implement typing, but ``typing_extensions``` will certainly be one of the dependencies required to do this in a way compatible with python 3.7 and even 3.8. While NumPy is making this an optional dependency, my hope is that it doesn't seem far fetched to make it a dependency here... 🤞 |
Just to add my $0.02, based on my experience with my own recent forays into static typing with Python, having I just wasn't aware that there was any concerted effort yet to make Astropy fully type-aware (though it seems like it would be a good thing to do incrementally. |
Agreed. Probably preaching to the choir here, but Mypy has incremental typing both in package scope and strictness. The easiest way to do typing would probably be to choose a subpackage (maybe a module in utils) that doesn't work with Units or Coordinates or anything that needs custom Astropy type hints and start building from there: get the |
I'd be interested in hearing any applicable case stories to show how spending (possibly considerable) time and effort to make astropy fully type aware would benefit. I'm not necessarily against working toward this goal, but I do think we need to have some justification for the effort and consider the costs. We do have to remember that many contributors to the core are not likely to be up to speed on writing typed code, so making the entry barrier for contributions even higher is a real concern. |
I share @taldcroft's fear, although I guess in a way it is just provides an easier-to-parse (computationally at least) method to express what we now do in the That said, I still like very much the implementation for p.s. numpy has gone the route of having a separate set of files (in |
I will say that I've become a pretty big fan of VS code + Pylance, and so I am beginning to see the benefit in my own workflow of type inference to allow for improve autocompletion and docs. I've actually been pretty amazed sometimes that it figures out some variable is a Table or whatever and gives me the right method auto-complete. From that perspective I'd be happy if there was a type annotation that said |
IMO there are a number of pros. Hopefully they outweigh the cons:
Agreed. This could be problematic. OTOH, in my experimentation with typed code, I've generally found that mypy and other static code analyzers have helped me a) improve my docstrings to be more accurate about function parameters and output and b) actually write clearer, more atomic code, since fancy tricks and Frankenstein functions make mypy unhappy. Some well-typed code may be easier to contribute to, while other functions will be harder. I think one of many arguments for mypy is that gradual typing will enable us to choose, at a pretty granular level, what to type hint. Code in active development we can leave untyped, if we so choose. Very mature code that really only gets occasional bug fixes can be fully typed.
That is certainly one option. I personally am not a huge fan of all the file duplication. But it's presumably is easier for other packages to find / build a typeshd. |
Slightly more on-topic, I actually do not have much of a problem with making |
I would also be +1 to having a separate hierarchy of type stubs. It's a bit of a pain to have to maintain two separate source hierarchies but it also would keep it out of the main codebase. One thing I don't like about typing and that kept me away from it a long time is having type hints everywhere really clutters the code up, especially if you're not used to reading python with type hints.
There's quite a number of facilities behind exactly this sort of use case. For the case of hstack for example, very schematically you might have something like: from astropy.table import Table, QTable
from astropy import units as u
from typing import TypeVar, List
T = TypeVar('T', bound=Table)
def hstack(tables: List[T]) -> T:
...
t1 = Table({'a': [1, 2, 3]})
t2 = QTable({'a': [1, 2, 3] * u.m}) # type: ignore
reveal_type(hstack([t1, t1]))
reveal_type(hstack([t2, t2]))
reveal_type(hstack([t1, t2]))
reveal_type(hstack([t1, 'asdf'])) Running mypy on this gives:
It finds an error on the last line because |
I think this specific issue will be resolved by #10662, which adds I think the related discussion on how to implement typing in astropy is well worth having and here might be the place.
@embray. I'm very far from an expert in typing, so I'm trying to determine if type stubs can be mixed with integrated type hints. The questions I'm trying to figure out are: can each subpackage make a separate decision and everything play nice, or can mypy etc only understand one format? Can the same be applied within a subpackage? |
|
Question
How should Astropy implement
typing
?What is the timeline?
What Infrastructure needs to be built to enable this? e.g. mypy checker in test suite or submissions to python's type-shed.
OLD Description
See #10662
The python
typing
library is under active development and new and useful features are not available for py3.6-3.8. Thetyping_extensions
library is now the official backport for all these types and is even developed within the main body of python (though offered as a separate install) — see https://github.com/python/typing/tree/master/typing_extensions.As astropy moves to add static typing, the extended types options, in addition to the guaranteed stability and long-term support, would make this a useful and safe dependency.
I'm making this an issue, not PR, because I know how contentious adding new dependencies can be.
Edit: this would probably not be a permanent additional dependency since the features in
typing_extension
all become core python. When new features stop being added regularly and our python minveraion catches up, this can be dropped.Edit: Now suggesting as an OPTIONAL dependency
The text was updated successfully, but these errors were encountered: