-
-
Notifications
You must be signed in to change notification settings - Fork 36
Implemented typehints for value parameter of IntervalDict #99
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
Implemented typehints for value parameter of IntervalDict #99
Conversation
Hi! Are there cases where a static type checker is helpful with those annotations? I mean, having |
Yes, there is a use for annotating keys with |
…ed amniguous overload method resolution
Oh, thanks, I wasn't aware of this difference between "object" and "Any". Considering that only this part of portion has types at the moment, would you consider moving this typing to a stub file? |
Sure. I have moved the typings to a typestub file Edit: Nevermind, theres something I have to fix |
Everything should be good now. I have added the type stub files and tested the library in my editor. |
Thanks, I'll have a look at it soon. Are there ways to check automatically the consistency of a stub file with its code? So that, if a change parts of portion, I can be warned by our CI that I need to keep the stub up to date? |
By "stub file", I meant avoiding type annotations within portion's code and to have them exclusively in a stub file :-) |
Okay! Sorry, I had some trouble earlier with where to place the stub files and their configurations in |
Thanks! I'm not experienced with stub files (nor with type hinting in general, I did it only once for another, larger library). Is there a need for https://github.com/AlexandreDecan/portion/blob/2f2c2e6a289bdb9efe104420b8a13b48ed23d07e/portion/__init__.pyi or can we simply provide Other related questions:
Thanks! |
I think you're right, we can remove I am also not an expert on the tooling surrounding typehints, so I don't have experience-based advice on typestub checking. I have tried using stubtest, and it reminded me of certain conflicts: The actual class However, I would like to point out the intent of type hints in the first place. Within your docstrings inside the methods, you have notes such as "Given X type, this returns Y" which is essentially part of what the typehints do, they are just like comments. I think it's reasonable to say that the functionality does not decide the typehints, but that the typehints are describe what the functionality does. From my experience, typehints are part of the planning stage. But in this case, IntervalDict already works perfectly, so I don't really know how we would check them. Aside from the overriding incompatibility with I don't necessarily have any plans for any other part. I was using For typeshed, I am not sure. I'm not an expert on the tooling surrounding typehints, but I think distributing types within this repository itself should work fine. Based on further research, typeshed seems to be a 'final stage' for types, meant for when a decent percentage of the library is typed and the types are tested. Not adding them to typeshed is more flexible and if there are other git issues in the future, it would be easier to resolve them if stubs are locally in this repository as opposed to typeshed. |
Would you mind if I merge this PR in a parallel branch, so that we can first cover the other parts before "officially" releasing these changes? |
Okay, sure |
Thanks!! :-) |
It is a big change the
IntervalDict
implementation. However, I believe this is useful to annotate the values even though the keys are somewhat unannotatable. For the keys however, I annotated them withobject
with the intent that users will use the correct type.