Open pslowinski-wn opened 5 months ago
@pslowinski-wn I don't fully grasp the issue you're reporting here. The service_agreement
property is only returned if its value is recipient
, otherwise it's not returned. So you have to check if the value is defined/present before checking the content I think
@remi-stripe Optional[X] is equivalent to Union[X, None].
. I should never get AttributeError: service_agreement
when calling service_agreement
on not None tos_acceptance
. The only possible values are str
or None
I hope it makes sense.
@pslowinski-wn Your code checks whether tos_acceptance
is None and you have to do the same for service_agreement
. For example this works for me:
if (
accountServiceAgreementRecipient.tos_acceptance is not None and
accountServiceAgreementRecipient.tos_acceptance.service_agreement is not None and
accountServiceAgreementRecipient.tos_acceptance.service_agreement == "recipient"
):
print(accountServiceAgreementRecipient.tos_acceptance.service_agreement)
I do think you have to handle the None
case before you can check the equality to a string like you were doing.
@pslowinski-wn I confirmed this is also covered in our wiki here under the Your type checker may require additional proof of safety
section.
@pslowinski-wn Your code checks whether
tos_acceptance
is None and you have to do the same forservice_agreement
. For example this works for me:if ( accountServiceAgreementRecipient.tos_acceptance is not None and accountServiceAgreementRecipient.tos_acceptance.service_agreement is not None and accountServiceAgreementRecipient.tos_acceptance.service_agreement == "recipient" ): print(accountServiceAgreementRecipient.tos_acceptance.service_agreement)
I do think you have to handle the
None
case before you can check the equality to a string like you were doing.
When I run this code, it only works for accounts with recipient
service agreement. For other accounts, I am getting the following exception:
File ".../.venv/lib/python3.11/site-packages/stripe/_stripe_object.py", line 172, in __getattr__
raise AttributeError(*err.args)
AttributeError: service_agreement
Damn you're right I'm sorry. I thought I thoroughly tested the examples but I double checked it and I tested on the wrong account. Right now the only solution I found is to use hasattr()
first. I'm checking with a few other people if there's a better alternative or not and will update you after!
I am doing the below:
if (
connected_account.tos_acceptance is not None
and getattr(connected_account.tos_acceptance, "service_agreement", None)
== "recipient"
)
However, it is rather a hack ;-)
@pslowinski-wn Okay so I think our types are incorrect for the case where a property is sometimes completely absent. Optional
means that is can be None
but not that it can be absent. Instead we should use NotRequired
for this. I got a bit confused because in other languages "optional" means "can be absent" and otherwise it's called "nullable" when it can return null
(what Python calls None
).
For now, I think using hasattr()
is your best approach like this
if (
accountServiceAgreementFull.tos_acceptance is not None and
hasattr(accountServiceAgreementFull.tos_acceptance, 'service_agreement') and
accountServiceAgreementFull.tos_acceptance.service_agreement is not None
):
print(accountServiceAgreementFull.tos_acceptance.service_agreement)
else:
print ("This account has no service_agreement property, so I assume it's 'full'")
And we're going to investigate changing the types to use NotRequired
but that will take a bit of time to scope and design. I'll keep your issue open in the meantime though!
I am definitely not a Python expert. However, NotRequired
doesn't look like a very client-friendly type. The client still would have to make a similar check to validate whether the field exists. It feels like providing a default None
value would be more client-friendly.
I don't disagree but we avoid doing that in our SDKs and we mirror what the API does instead. It's a common pattern to have some properties really be optional. The most canonical example would be the PaymentMethod API where you have type: 'card' | 'klarna' | 'sepa_debit' | ...
and then based on the value of type
you also have a property with that name with more details so card: {....}
or klarna: {....}
but only one returned, everything else is not returned.
But I agree it can make it awkward in some languages and that's what we're going to investigate further.
And to be clear, I've never liked that service_agreement
is not returned for full
. I've been trying to get it fixed internally since it launched but we haven't prioritized that fix just yet. But in this specific case I think service_agreement
should neither be null or optional and should always return the exact value.
If we wanted to change the types to match what happens at runtime, we would need NotRequired
-- except that's not actually possible with the state of Python types today, because NotRequired
only exists for TypedDict
types and StripeObject
is an actual class.
If we wanted to change the runtime to match what's described by the types, your suggestion
It feels like providing a default None value would be more client-friendly is what would accomplish this. But this would be a very breaking change (too breaking for us to consider, I think), as it would cause code
if hasattr(account.tos_acceptance, "service_agreement"): # Assume it's there and set to an actual value else: # It's not there
to break and need to change to an
account.tos_acceptance.service_agreement is not None
check.
We could consider making "absent as none" behavior opt-in, and recommend this to anybody who uses types -- which I am experimenting with in https://github.com/stripe/stripe-python/pull/1229. Then it would be
stripe.absent_as_none = False
if (account.tos_acceptance.service_agreement is not None):
# this would no longer raise
and this would make the runtime behavior match what's described in the types. But this seems hard to discover and doesn't feel like a true fix, and the types would still be inaccurate for anybody unable to discover this option.
Another option would be to leave the possibly-completely-absent fields at runtime, omit them from the type definitions, and expose properties or getters with a different name and the desired semantics, e.g. instead of
>>> account.tos_acceptance.service_agreement
KeyError: 'service_agreement'
you would do
>>> account.tos_acceptance.get_service_agreement()
None
or maybe
>>> account.tos_acceptance.service_agreement_value
None
but so far I can't really think of any naming scheme that would make this feel good.
We could consider making "absent as none" behavior opt-in, and recommend this to anybody who uses types -- which I am experimenting with in https://github.com/stripe/stripe-python/pull/1229. Then it would be
I like that idea. I would also hope absent_as_none=True
will be a default with future releases.
Note: I come from the JVM world, so I have some biases :D
I opened a "typing" discussion on the Python discourse about this issue to see if anybody there has any ideas.
https://discuss.python.org/t/typing-notrequired-like-for-potentially-absent-class-attributes/46963
If not, I think probably my preferred approach for addressing this is the add additional getters I described, which we would pursue if enough users were running into this and having trouble.
Describe the bug
Account
class hastos_acceptance: Optional[TosAcceptance]
.TosAcceptance
class hasservice_agreement: Optional[str]
The following code:
Should not rais exceptions, but it does: AttributeError: service_agreement
To Reproduce
if
statementtheExpected behavior
Exception should not be raised
Code snippets
No response
OS
macOS
Language version
3.11.4
Library version
8.1.0
API version
2019-08-14
Additional context
No response