-
Notifications
You must be signed in to change notification settings - Fork 0
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
allow any iterable of strings #1
allow any iterable of strings #1
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.
Fair point, I guess that will cover other iterable cases. Just one thing but otherwise, LGTM
@@ -11,3 +11,4 @@ certifi>=2023.11.17 | |||
cython>=3.0.8 | |||
cymem>=2.0.8 | |||
orjson>=3.9.10 | |||
typing-extensions>=4.7.0 |
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.
Even if it's already there as a transitive dependency of the WebHost (for Flask-Limiter), I'm not a fan. TypeGuard
is available in typing
since python 3.10, so I would rather have this marked as required for python less than 3.10.
I think something like that would work:
typing-extensions>=4.7.0; python_version < '3.10'
Since typing-extensions won't be an explicit dependency for 3.10+, since it could be removed from transitive dependencies, TypeGuard
could be imported with something like
try:
from typing import TypeGuard
except ImportError:
from typing_extensions import TypeGuard
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.
That seems like adding complication for no benefit.
It's not like the usefulness of typing_extensions will go away at 3.10
There are also new features in 3.11 that would be useful (Required
NotRequired
Self
)
and 3.12 (@override
) and 3.13 (ReadOnly
)
and there will be usefulness in 3.14, 3.15, 3.16...
No matter what version of Python we're on, it will be useful.
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.
Well, then I guess there is no point fighting against this dependency
2954a7d
into
Jouramie:core/allow-more-iterables-in-option-parsing
What is this fixing or adding?
Do we want to allow any iterable of strings?
Note that typing-extensions isn't a new dependency. It's already a dependency of other packages we depend on. It's just better to be explicit if we use it directly (in case we remove the other dependencies that depend on it).
How was this tested?
just unit tests