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

Integer field won't accept boolean value when strict is False #1475

Open
mxamin opened this issue Dec 17, 2019 · 2 comments · May be fixed by #1476
Open

Integer field won't accept boolean value when strict is False #1475

mxamin opened this issue Dec 17, 2019 · 2 comments · May be fixed by #1476

Comments

@mxamin
Copy link

mxamin commented Dec 17, 2019

As mentioned in the documentation, for Integer fields when the strict parameter is False any value that is castable to integer is acceptable:

strict – If True, only integer types are valid. Otherwise, any value castable to int is valid.

But actually, if you pass True or False when strict is False the validation will fail although boolean values are castable to int:

assert int(True) == 1
assert int(False) == 0
@lafrech
Copy link
Member

lafrech commented Jan 12, 2020

True and False are cast consistently by int, float and decimal.Decimal.

They are explicitely rejected in Number._validated.

I'd rather we keep a consistent interface here for all number fields. Accepting booleans could be set by yet another field init parameter. Not sure.

Or maybe extend strict to all number fields and reject booleans if strict is True for all number fields alike.

Not sure this ought to be in Number, though. Just because all currently child fields auto-cast, does not mean any child field would. What about a custom field inheriting from Number but for a type that cannot cast booleans.

Overall, I'm wondering whether we should fix the code or the docs.

@mxamin
Copy link
Author

mxamin commented Jan 17, 2020

Actually the strict option doesn't do the job as it should be. For example, if you set strict=True, Integer field will still accept the float type. So removing that parameter entirely can also be an option.
But of course, if you want to keep the backward compatibility we should fix it.

I think extending that parameter to Number is not a good idea since it has no use in Decimal field which is inheriting from Number

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 a pull request may close this issue.

2 participants