Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Use of numbers.* encounters a bug in mypy #9

Open
jpmckinney opened this issue Nov 22, 2021 · 6 comments · Fixed by baodrate/types-pika#9
Open

Use of numbers.* encounters a bug in mypy #9

jpmckinney opened this issue Nov 22, 2021 · 6 comments · Fixed by baodrate/types-pika#9

Comments

@jpmckinney
Copy link

python/mypy#3186

Could Union[int, float] be used instead of numbers.Real?
@posita
Copy link

posita commented Dec 15, 2021

I don't know much about your use case (and don't even know if it's appropriate for a stubs library like this one), but you might want to see if numerary can help you. If this journey lines up with your own, numerary could be an intermediary solution until python/mypy#3186 is fixed. (Alternatively, the techniques used therein might provide inspiration if you can't take a dependency.) Happy to consult here, if helpful.

@baodrate
Copy link

baodrate commented Jan 16, 2022 •
edited
Loading

I think numerary would be overkill here. The types pika supports are clearly documented as e.g. int | float | None and I don't see a particularly good reason why it needs to be expanded beyond that (especially since there's no indication that the upstream package supports anything beyond the basic concrete types).

edit: I see that internally pika does check against numbers.Integral/numbers.Real. That justifies the typing as correct, IMO. Though pulling in a dependency as a stubs module still seems very iffy to me. I'd rather opt to just support the concrete types as a compromise for usability.

@jpmckinney
Copy link
Author

I'd rather opt to just support the concrete types as a compromise for usability.

I'd be happy with that compromise!

baodrate added a commit to baodrate/types-pika that referenced this issue Jan 17, 2022
@baodrate
linting: Fix rest of stubtest
7071f2b 
This commit fixes the issue pointed out in:
hahow/pika-stubs#9

Namely `int` and `float` not (statically) evaluating as subclasses of
their abstract types in `numbers`:
python/mypy#3186

These abstract types have been replaced with their concrete equivalent.
Note that this does not match the code in pika, which *does* compare
against e.g. `numbers.Integral` at runtime. But the documented API only
mentions concret types (i.e. `int` and `float`) so this is considered a
reasonable compromise.

addresses hahow/pika-stubs/issues/9
@baodrate baodrate mentioned this issue Jan 17, 2022
Merged
@baodrate
Copy link

baodrate commented Jan 17, 2022 •
edited
Loading

FYI, I'm resolving this and the other outstanding issues in my fork, since the maintainer has been unresponsive and their github account no longer exists. (I assume they no longer work for hahow).

Fixed up all the inconsistencies and incorrect type annotations I could find. Running mypy on our private codebase with the updated stubs seems to indicate they're reasonably correct. Not sure yet what I'll do about releasing, since the pika-stubs name is taken.

@jpmckinney
Copy link
Author

I'll try it! ( https://pypi.org/project/types-pika/ )

@jpmckinney
Copy link
Author

The issue with numbers is fixed. I opened another issue: baodrate/types-pika#11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Linting & Testing baodrate/types-pika
3 participants
@jpmckinney @posita @baodrate