-
Notifications
You must be signed in to change notification settings - Fork 592
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
bug: Incomplete type hints #6844
Comments
@binste Thanks for the issue! Yeah, we're not testing mypy at the moment. The vast majority of ibis was implemented before type hints became widespread and it's a lot of work to add them. I know @jcrist has thoughts here. I think there may be a way we can ensure that our type hints are as-expected without having to annotate the entire codebase. |
Hopefully this isn't a blocker for you, but let us know! |
I can relate to that. It's the same story for Vega-Altair where I'm currently in the process of typing at least the public API and it's not easy...
I think the issue title I chose is misleading. Mypy allows for gradually typing existing codebases and so it's fine if Ibis is not completely typed. Issues arise when the provided type hints are either wrong (e.g. specified types on function input arguments cannot be processed by function) or incomplete (e.g. a union of types does not cover all accepted types) where the later one seems to be the case for the 3 errors I mentioned above. The process which we follow for Altair and what I've seen with other libraries is that
By introducing the Based on #2823 and the related PRs, I take it that it's not easy to fix all existing mypy issues. For what it's worth, I think you could either remove again the Running |
Happy to help out with reviewing PRs or creating some smaller ones myself to move this forward! Curious to hear what your thoughts are and your past experience with type hints in this codebase. |
I'll document here some mypy errors where I'm not familiar enough with Ibis to create a PR:
|
Thanks @binste for working on this! I'm going to take a closer look to the type errors you mentioned. |
Thank you @kszucs! Here's another: There are a lot of
from typing_extensions import dataclass_transform
@dataclass_transform()
class Foo:
a: int
b: str = "default"
class Bar(Foo):
c: str
d: int = 0
class Baz(Bar):
e: int
f: str = "default" Gives an error on
A solution could be to use |
It can be used with both positional ibis/ibis/common/annotations.py Line 199 in 90e852d
|
I just ran I don't think any of the maintainers is up for the herculean effort of fixing all of those. My inclination is to remove the Thoughts? |
@tswast I think you added this. Can you recall why it was added? |
Sounds reasonable to me. We started using Ibis extensively in our code base but as we use mypy, we have to use quite a few |
We also use mypy in BigQuery DataFrames, which is why I added this in the first place. We do have a good handful of typing.cast calls, but overall I find the benefits are outweighing the limitations. |
I think this gradual plan makes sense to me. #6844 (comment) In the places where the type hints are wrong, we should either remove them or fix them. |
A tad more context, I'd like to be able to use type hints so that VS Code (Pylance) autocomplete has some chance of being helpful in my functions that take and return Ibis expressions. For this purpose it has been helpful, even though the types aren't perfect. |
Indeed, I also find this very useful in VS Code! Luckily, Pylance uses type hints for autocompletion even if no py.typed file is present for a library. (We see this for Vega-Altair, where Pylance uses the hints for autocompletion) |
Thanks for the context all. In my reading of the above, I don't see a benefit to having the |
If someone would like to push this forward, please submit incremental pull requests. At the moment we're not going to take on the task of making our type hints complete. Please open up bug reports for incorrect annotations as they arise! |
What happened?
While evaluating Ibis at work, I found some unexpected mypy errors:
I'll keep adding to this issue in case I'd find more. Also, a big thank you for making Ibis a typed library! :)
What version of ibis are you using?
Ibis 6.1
Mypy 1.4.0
Python 3.11
What backend(s) are you using, if any?
No response
Relevant log output
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: