Skip to content

Add SearchableAPIResource and SearchResultObject for types-stripe #8696

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

Merged
merged 12 commits into from
Sep 13, 2022

Conversation

yejia-stripe
Copy link
Contributor

Add type annotations for SearchableAPIResource and SearchResultObject for the stripe library stubs, and add SearchableAPIResource inheritance for relevant classes.

Fixes stripe/stripe-python#871

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

Are some of these things new in stripe v4? If so, you may need to bump the version field in METADATA.toml to get stubtest to pass (but that might also add some more errors to CI — see #8490)

@github-actions

This comment has been minimized.

@yejia-stripe
Copy link
Contributor Author

yejia-stripe commented Sep 6, 2022

@AlexWaygood The overall feature isn't new to v4, but the way I'm typing it is not technically accurate? The search and search_auto_paging_iter don't actually exist on SearchableAPIResult, but I didn't want to type each subclass separately. They will be defined after I release stripe/stripe-python#873, but that would be for v4.

I would like to make this change without updating to v4. Would I be able to add an exception for this for now? Or should I add types for each subclass?

@AlexWaygood
Copy link
Member

I would like to make this change without updating to v4. Would I be able to add an exception for this for now? Or should I add types for each subclass?

Yeah that should be fine! Sorry for the delayed response!

We do that for a few other classes, e.g.:

importlib.abc.MetaPathFinder.find_spec # Not defined on the actual class, but expected to exist.
importlib.abc.PathEntryFinder.find_spec # Not defined on the actual class, but expected to exist.

You'll just need to add some entries to https://github.com/python/typeshed/blob/master/stubs/stripe/%40tests/stubtest_allowlist.txt to get the CI happy with it :)

@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly looks good! Just one suggestion, and two questions

@github-actions

This comment has been minimized.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!

@yejia-stripe
Copy link
Contributor Author

Thank you for the help!! :)

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit 42c044e into python:master Sep 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.

mypy does not recognize SearchableAPIResource
2 participants