Invalid is check in credit card validation

[mike-hart]

https://github.com/samuelcolvin/pydantic/blob/master/pydantic/types.py#L669

if card_number.brand is (PaymentCardBrand.visa or PaymentCardBrand.mastercard)

Assuming brand was reduced to the integers, and we had 3 brands, then this is the equivalent of:
3 is (1 or 2), which you'd hope were false, 1 is (1 or 2) which you'd hope were true, and 2 is (1 or 2) which you'd also hope were true. However, or short-circuits, so these reduce to 3 is 1, 1 is 1, and 2 is 1. This means the preferred outcome for the first two cases works, but it would fail for mastercard.

Perhaps this should be

if card_number.brand is PaymentCardBrand.visa or card_number.brand is PaymentCardBrand.mastercard:

I think this was added relatively recently - 0c57937#diff-470d0730db2e82eaaf27ad3655a667b3R582 - so hopefully it hasn't affected too many people.

[StephenBrown2]

Probably works because of some Enum class magic, but could be reworked to:

if card_number.brand in (PaymentCardBrand.visa, PaymentCardBrand.mastercard)

[mike-hart]

I guess it depends if equality is acceptable compared to identity?

[mike-hart]

I don't think magic works with is, and I'm kinda glad of that:

>>> from enum import Enum
>>> class CardType(Enum):
...     Diners = 1
...     Visa = 2
...     MasterCard = 3
...
>>> CardType.Visa is (CardType.MasterCard or CardType.Visa)
False

[StephenBrown2]

Good to confirm.

[samuelcolvin]

Assuming this is resolved.

[mike-hart]

Sadly not resolved, code is still

if card_number.brand is (PaymentCardBrand.visa or PaymentCardBrand.mastercard):

which won't work if card_number.brand is PaymentCardBrand.mastercard.

[StephenBrown2]

Yes, I was just confirming the logic is broken. @mike-hart you might want to submit a PR referencing this issue with the fix.

[samuelcolvin]

The use of x is (y or z) is just a mistake, it simplifies to x is y if y is truey or x is z otherwise. In this case since PaymentCardBrand.visa is truey we're effectively just doing brand is PaymentCardBrand.visa.

I'll replace it with an in check.

A bit more background on enums:

• Identity checks work with the actual enum member but not with the enum value
• equality checks work with both the enum member and the enum value, but only if the enum inherits from the correct type as well as Enum.

class PaymentCardBrand(str, Enum):
    amex = 'American Express'
    mastercard = 'Mastercard'
    visa = 'Visa'
    other = 'other'

b = PaymentCardBrand.visa
b is PaymentCardBrand.visa
#> True
b == PaymentCardBrand.visa
#> True

b = 'Visa'
b is PaymentCardBrand.visa
#> False
b == PaymentCardBrand.visa
#> True (This would not work with the current PaymentCardBrand which doesn't inherit from str)

I don't know if card_number.brand can be a string or only a PaymentCardBrand, but better to fix anyway.

[mike-hart]

Ah, either way, I added this - #1320 - but since you propose a different approach I guess it's less valuable now.

Thanks for responding/reopening.