-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
provide type annotations #93
Comments
Hiya @jab - I'd been looking for an excuse to dig into Python's type hints, do you mind if I work on this over the weekend? Or are you waiting until Python 3.5 hits end-of-life in September 2020? |
Hi @cjgibson, I'd be delighted for you to work on this! I actually haven't been sure how many users were even interested in having type hints in bidict. Is your interest in adding them just an excuse to learn type hints better, or would you benefit from them as a bidict user too? Always happy to hear from users and want to make sure users' needs are taken into account for future releases. I think I'm fine with adding this anytime, and dropping support for 3.5 before its EOL if that'd make this easier. As usual, I think dependents (especially those stuck on older versions of Python) should be pinning to known-compatible versions in production anyway. That said, it may make sense to remove Python 2 support (#97) before working on this (which also doesn't have to wait for Python 2 EOL). That would make all future changes easier, and especially changes like this, I think. I'm pretty busy and don't know how soon I'd be able to review your PR. (Especially since I also haven't yet had time to become an expert on Python's type hints, which I'd want to first.) But receiving a high-quality PR would be a great motivation to make time for this. If you decide to work on this (and/or #97), please feel free to keep in touch during the process in case there's anything I can help with as time permits. And thanks so much for your interest in contributing to bidict! |
To be totally honest, I had assumed bidict would already have them - I've used this library in the past as a great learning library, so I'm a little excited to get a chance to improve bidict in that regard!
Totally agreed. I'll start on that over the weekend instead, and keep this in the back of my mind in the interim.
Absolutely no worries - comes with the territory 😀 |
Hey @cjgibson, I felt bad suggesting you work on the much less interesting #97 before looking at type hints, so I made some time this morning to knock out #97. |
Thank you for handling that! I got started late Sunday: jab:50e87fd...cjgibson:9d76098 But only got my fork to a sane state this evening. An unexpected issue is that mypy really doesn't like mixing Generics with |
Wow! Thanks @cjgibson, fantastic to see your progress! 😻 I pulled your changes into my Thanks again for working on this! So cool! 😊 |
Hiya @itaisteinherz, you're welcome to take this on if you'd like! My initial work brought about some refactoring changes from @jab, so I stepped back a bit while those were ongoing and never really came back around to this task. From what I recall @jab has started adding their own typing over on the typehints branch, but I'll let them speak to that. |
Thanks for your interest @itaisteinherz, and thanks for your update, @cjgibson! Indeed, I continued work on this on my typehints branch after the great start @cjgibson made here, but unfortunately I hit a wall with limitations in mypy and haven't gotten back to it since. The biggest problem I hit is that I think mypy currently makes it impossible to type hint the f = frozenbidict({0: 'zero'})
reveal_type(f.inverse) and have that In other words, for any Thanks again for your interest, and looking forward to getting type hints in bidict hopefully before too much longer. |
Just a quick update to say that I posted this example to python/typing#548 and Guido said it was helpful. |
What is the recommended type annotation for bidict right now? |
Every time I try to pick this back up, I hit another challenge. This time it appears to be a bug in Python 3.6. Reported here: https://bugs.python.org/issue41451 Until I can drop support for Python 3.6, I think we're blocked on finding a workaround for this bug. |
Big thanks to @oremanj for suggesting a workaround for https://bugs.python.org/issue41451. (In case of interest, I had to adapt the full-on Opening a PR momentarily that should resolve this issue when it's merged. 🎉 |
Hi all, I just merged #112 adding type hints! Could you please run |
@jab hey, great project. I just tried the fix and it works with >>> from bidict import bidict
>>> a: bidict[str, int] = bidict({"a": 5}) |
Thanks for letting me know, @HarrySky! I'm currently trying to figure out whether I should be using a covariant value type (following the example from UPDATE: I was pretty sure before, but now I'm convinced that as long as Mapping isn't promising a covariant key type, BidirectionalMapping can't promise a covariant value type, since it would imply the existence of a BidirectionalMapping with a covariant key type (namely, its inverse). |
@cjgibson, @itaisteinherz, @MartinThoma, @HarrySky, et al., just released v0.21.0, the first bidict release with type hints! Thanks for the testing and encouragement here. 😊 Hope the new release works well for you, and please let me know if you have any issues. |
Maybe after removing support for older versions of Python.
Ref:
The text was updated successfully, but these errors were encountered: