-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Support reading annotated variables #107
Comments
Nice. Thanks for documenting the motivation and especially finding prior art. I'd not thought to look at I do see the value in supporting valid syntax. Especially since this library is being shared for more than just the On the other hand, there's lots of valid Python syntax that this construct doesn't support, such as mutation ( I'm a little worried that nobody has asked for this feature. Are you wanting for yourself to add type annotations to a I'm also apprehensive as it starts to violate the zen of "preferably one obvious way to do it". Overall, I'm inclined to say we should do it, especially if the functional approach can be maintained (such as drafted in c9a7094). I promise to be more forgiving for another PR. |
But does it? I do believe that we could say there is no more ways to do it with this feature: the obvious way to specify requirements without passing them via Syntax isn't what violates the zen here, I believe. I think it's just about some basic flexibility, but that's an opinion, not a helpful fact. In other words, what I mean is: __requires__: list[str] = ["jaraco.functools"] and __requires__ = ["jaraco.functools"] are essentially identical, so why not support both, given the cost is so low? |
Yes. Additionally, we do not support </offtopic>
Well, let me be the first one then. I wanted to declare a requirement list without overspecifying the type, that is And I'm pretty sure some people would use it. But I can't speak for them. |
Alright, see you there! |
Consider using c9a7094 as a baseline (though not required). |
Annotating
__requires__
/__index_url__
might be discouraged becausepip-run
only supports literal assignments of__requires__
/__index_url__
.This issue aims to advocate for supporting this case nonetheless:
or this case
and so on.
Since
__requires__
(and__index_url__
as well) is expected to be an immutable literal defined in one place, I'd suggest:typing.Final
which signals: "hey, you shouldn't be reassigning this name__requires__
here"__requires__
), e.g.__requires__: Final[Sequence[str]]
(not a huge fan becausestr
is itself aSequence[str]
, but it correctly matches all types of valid__requires__
values) or__requires__: Final[tuple[str, ...]]
which I think best resembles strings' atomicity.This practice has a bunch of users. Let's check for
__all__
, which is a common package-related module-level dunder name, typically not expected to change dynamically:This feature:
Moreover, it might increase general type safety knowing that
__requires__
can either be a collection of strings or a single string, treated atomically.I do think this feature is in
pip-run
's philosophy and makes its usability wider at not much of a cost.The text was updated successfully, but these errors were encountered: