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.

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

Parsimonious stubs #7477

Merged
merged 18 commits into from
Mar 13, 2022
Merged

Parsimonious stubs #7477

merged 18 commits into from
Mar 13, 2022

Conversation

jpy-git
Copy link
Contributor

@jpy-git jpy-git commented Mar 12, 2022

Parsimonious is a PEG parser package.

PyPI link: https://pypi.org/project/parsimonious/

I'm making use of this package in a couple projects and there's also an open issue about adding type hints so figured I'd pick this up.

stubs/parsimonious/parsimonious/exceptions.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/__init__.pyi Show resolved Hide resolved
stubs/parsimonious/parsimonious/expressions.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/expressions.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/grammar.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/nodes.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/nodes.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/nodes.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/nodes.pyi Outdated Show resolved Hide resolved
stubs/parsimonious/parsimonious/nodes.pyi Outdated Show resolved Hide resolved
@jpy-git
Copy link
Contributor Author

jpy-git commented Mar 13, 2022

@JelleZijlstra Thanks for the thorough review 😄

I believe I've addressed the comments. I'm having some trouble with getting Check third party stubs with stubtest to pass since it's complaining about some of the subclass types not agreeing with the superclass types, which is what's happening in the code, and stubtest_allowlist doesn't seem to fix things. Any ideas what I'm missing?

@hauntsaninja
Copy link
Collaborator

stubtest runs a mypy build first to understand what's in the stubs. The mypy build is failing, so we haven't even gotten to the actual stubtest logic, which is what stubtest allowlists affect.

Assuming your subclass types are actually correct, adding type ignore will allow the build to proceed. For the "Type application has too many types", it's probably some mypy bug, try using an old style typing.Union + typing.Tuple

@JelleZijlstra
Copy link
Member

The errors are from mypy itself, not stubtest; they'll need # type: ignore. I'll try to figure out why the regular mypy test didn't catch this.

@JelleZijlstra
Copy link
Member

@hauntsaninja it's not expected that the mypy test in CI is passing though, right? Looks like it runs only on the stdlib.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Mar 13, 2022

Yeah, looks like mypy_test is only running against the stdlib. It's not the first time I've seen stubtest third party catch mypy errors that mypy_test hasn't caught, so the test has probably been buggy for a while.

@JelleZijlstra
Copy link
Member

It's likely related to the mypy feature where it does os._exit to exit faster.

@JelleZijlstra
Copy link
Member

Fix in #7478

@JelleZijlstra JelleZijlstra merged commit 3a9abf8 into python:master Mar 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants