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

Fix StringStore.__getitem__ return type depending on parameter types #10741

Merged
merged 2 commits into from
May 3, 2022

Conversation

ldorigo
Copy link
Contributor

@ldorigo ldorigo commented May 2, 2022

Small fix using @overload so that StringStore.__getitem__ returns an int when given a str or bytes and a str when given an int.

Types of change

Small typing improvement

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

Small fix using  `@overload` so that `StringStore.__getitem__` returns an `int` when given a `str` or `bytes` and a `str` when given an `int`.
@adrianeboyd
Copy link
Contributor

This change makes sense, but it looks like mypy doesn't support it, at least not in the version we're currently using (until thinc updates are complete to support newer mypy and pydantic version). Does the mypy check look different on your end?

@adrianeboyd adrianeboyd added the types Issues related to typing or typing tools label May 2, 2022
@ldorigo
Copy link
Contributor Author

ldorigo commented May 2, 2022

Sorry, I haven't tested it in mypy (only in pyright), and I didn't think that types were part of the tests (I just submitted the PR as a small patch via Github's online editor). I did check and it seems like @overload is used in other parts of Spacy though - will get a proper dev environment locally now and see what's up :-)

spacy/strings.pyi Outdated Show resolved Hide resolved
@ldorigo
Copy link
Contributor Author

ldorigo commented May 3, 2022

Sorry, had some trouble compiling locally - thanks for fixing it yourself!

@adrianeboyd adrianeboyd merged commit 0a92d56 into explosion:master May 3, 2022
danieldk pushed a commit that referenced this pull request Jun 7, 2022
…10741)

* Fix StringStore.__getitem__ return type depending on parameter types

Small fix using  `@overload` so that `StringStore.__getitem__` returns an `int` when given a `str` or `bytes` and a `str` when given an `int`.

* Update spacy/strings.pyi

Co-authored-by: Adriane Boyd <adrianeboyd@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
types Issues related to typing or typing tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants