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

phrasematcher.pyi has the wrong type annotation for docs parameter in add #10234

Closed
ramonziai opened this issue Feb 8, 2022 · 5 comments · Fixed by #10235
Closed

phrasematcher.pyi has the wrong type annotation for docs parameter in add #10234

ramonziai opened this issue Feb 8, 2022 · 5 comments · Fixed by #10235
Labels
feat / matcher Feature: Token, phrase and dependency matcher types Issues related to typing or typing tools

Comments

@ramonziai
Copy link
Contributor

ramonziai commented Feb 8, 2022

How to reproduce the behaviour

Typecheck any (type-annotated) code that uses PhraseMatcher.add() with mypy. You'll get something like:

error: List comprehension has incompatible type List[Doc]; expected List[List[Dict[str, Any]]]

mypy is correct because line 17 of phrasematcher.pyi actually reads docs: List[List[Dict[str, Any]]],, but it should probably be docs: List[Doc] according to https://spacy.io/api/phrasematcher#add.

Your Environment

  • Operating System: Ubuntu 20.04 (on GitHub Actions)
  • Python Version Used: 3.8
  • spaCy Version Used: >= 3.1.4
@adrianeboyd adrianeboyd added feat / matcher Feature: Token, phrase and dependency matcher types Issues related to typing or typing tools labels Feb 8, 2022
@adrianeboyd
Copy link
Contributor

Hi Ramon! I'd come across this too and have a fix in an adjacent PR (#9880), but it's not clear when that will be merged, so we can fix this separately for the next patch release instead. If you'd like to submit a PR you are welcome to, but I can also do it if you'd prefer.

@ramonziai
Copy link
Contributor Author

Hey Adriane 😄 Ok sure, I'm happy to submit a PR. The guidelines say I should write a failing test for the bug first, can I skip that since it's more of a "meta problem"?

@adrianeboyd
Copy link
Contributor

Yeah, the types aren't checked directly by the unit tests anyway, so you can skip writing a test for this.

@ramonziai
Copy link
Contributor Author

See #10235.

@svlandeg svlandeg linked a pull request Feb 8, 2022 that will close this issue
3 tasks
@github-actions
Copy link
Contributor

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat / matcher Feature: Token, phrase and dependency matcher types Issues related to typing or typing tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants