Skip to content

Add overload annotation for stripe DeletableAPIResource delete classmethod #7230

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

Conversation

yejia-stripe
Copy link
Contributor

Fixes stripe/stripe-python#770

stripe's DeletableAPIResource.delete instance method has a wrapper that effectively overloads the method to add a class method version.

Both

s = stripe.Subscription.create(...)
s.delete()

and

s = stripe.Subscription.create(...)
stripe.Subscription.delete(s.id)

should work, but the latter raises a type error.

@JelleZijlstra
Copy link
Member

I fixed the lint error, but it looks like pytype is unhappy about having @classmethod on only one of the two overloads. Could you rewrite this so the decorators are consistent?

@JelleZijlstra
Copy link
Member

Actually there's another lint error too—you should use _typeshed.Self.

@yejia-stripe
Copy link
Contributor Author

I fixed the lint error, but it looks like pytype is unhappy about having @classmethod on only one of the two overloads. Could you rewrite this so the decorators are consistent?

@JelleZijlstra I want to fake having a class method and an instance method with the same name, which is weird and it makes sense that pytype complains about it. I'm not sure how to both express that without using @classmethod for just one of the overloads. Do you have suggestions? Thanks!

@JelleZijlstra JelleZijlstra self-assigned this Feb 22, 2022
@JelleZijlstra JelleZijlstra self-requested a review February 22, 2022 16:35
@JelleZijlstra JelleZijlstra removed their assignment Feb 22, 2022
@JelleZijlstra
Copy link
Member

Would annotating it as only a classmethod work? Classmethods can still be called on instances.

@srittau
Copy link
Collaborator

srittau commented Feb 24, 2022

I think it's easiest to add the following to @tests/stripe/stubtest_allowlist.txt to fix CI: stripe\..*\.delete (untested).

yejia-stripe and others added 2 commits February 25, 2022 10:06
…rce.pyi

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@JelleZijlstra JelleZijlstra merged commit ee8aa1e into python:master Feb 25, 2022
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 6, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 6, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 6, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 6, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 7, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 7, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 7, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 7, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 8, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 8, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
PIG208 added a commit to PIG208/typeshed that referenced this pull request Aug 8, 2022
Similar to python#7230, stripe.PaymentIntent.confirm can be called as an
instance method or a classmethod.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
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.

Issue with typeshed's stripe types in stripe.Subscription.delete
3 participants