Make bool_validator strict #617

dmontagu · 2019-06-23T09:10:25Z

Change Summary

Make bool_validator a little more strict, in line with #579

In particular, allow True, False, 1, 0, all yaml true/false strings, the strings 't'/'T'/'f'/'F' (following DRF), and the bytes versions of those strings. Anything else raises a ValidationError.

Also adds RelaxedBool to retain current functionality if desired.

Related issue number

#579

Reviewing the discussion in #579, it wasn't clear to me whether the goal was to modify StrictBool to allow these strings, or to change bool_validator. My current interpretation is that the goal was to change bool_validator to bring it in line with more expected/useful behavior in a non-python context. If that's wrong, I'd be happy to refactor so that this logic applies to StrictBool instead.

This would be a breaking change for a very common type (if modifying bool_validator anyway), so I wouldn't necessarily expect it to be merged prior to a 1.0 release, but figured I'd start the process.

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
HISTORY.rst has been updated
- if this is the first change since a release, please add a new section
- include the issue number or this pull request number #<number>
- include your github username @<whomever>

pydantic/validators.py

codecov · 2019-06-23T09:16:29Z

Codecov Report

Merging #617 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #617   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2710   2718    +8     
  Branches      532    534    +2     
=====================================
+ Hits         2710   2718    +8

tests/test_types.py

samuelcolvin

Otherwise I'm happy with this.

@cazgp, you created the issue. Feedback here welcome.

pydantic/validators.py

cazgp · 2019-06-26T13:17:21Z

Thanks for this!

tests/test_types.py

dmontagu · 2019-06-28T09:05:02Z

@samuelcolvin I've updated the docs. However, I still have two questions:

should I update the history? I'm not sure if the goal is to include this in an upcoming 0.x release or wait for 1.0
I only made note of the change in the section of the docs about StrictBool and RelaxedBool. I think this is okay, but given this is a breaking change to the behavior of bool, should a note be placed somewhere else as well?

lgtm-com · 2019-06-28T09:13:59Z

This pull request introduces 1 alert when merging fc41a25 into ccdf8e1 - view on LGTM.com

new alerts:

1 for Module-level cyclic import

lgtm-com · 2019-06-28T09:36:33Z

This pull request introduces 1 alert when merging 1a73534 into ccdf8e1 - view on LGTM.com

new alerts:

1 for Module-level cyclic import

samuelcolvin

Looking pretty good, RelaxedBool seems good.

See #576 (comment) for discussion on how we deal with history/releases.

samuelcolvin · 2019-06-28T09:36:02Z

docs/index.rst

+
+If you want more permissive behavior, ``RelaxedBool`` can be used to cast values to bool using standard python logic.
+For example, ``None`` would be cast to False. Note that this behaves differently than the standard ``bool`` field
+for some values: for example, a ``RelaxedBool`` field would cast the string "false" to True.


Suggested change

for some values: for example, a ``RelaxedBool`` field would cast the string "false" to True.

for some values: for example, a ``RelaxedBool`` field would cast the string ``"false"`` to ``True``.

docs/index.rst

pydantic/validators.py

docs/index.rst

samuelcolvin · 2019-08-06T15:22:02Z

please can you rebase and move the history change to changes/ as per #719.

HISTORY.rst

docs/examples/exotic.py

docs/index.rst

pydantic/errors.py

samuelcolvin · 2019-08-08T08:57:39Z

pydantic/types.py

@@ -243,6 +244,21 @@ def validate(cls, value: str) -> str:
        return validate_email(value)[1]


+class RelaxedBool(int):


humm, I'm not sure Relaxed is a good choice of word.

I personally think this type is very wierd and people shouldn't use it since it makes absolutely zero sense to parse 'false' to False.

I think either we should:

rename it to something like StandardLibBool and recommend against using it

or, (better in my opinion) remove it altogether. Please can implement it themselves if they really want it.

I think the only reason to use it would be performance; right now if you want speed when you know you are getting the right type, it's awkward. But I agree it would be better if the name reflected this.

Actually I guess the default validator is pretty fast if it is already the right type?

I'd be okay either dropping or giving a different name. (I'll keep my commits locally for when someone asks for it 😈)

okay, let's keep it but say that in the docs.

@samuelcolvin I did some minor investigation of the timing and I think the speed difference between the two bool validators is negligible (when the inputs are bools); in my runs I didn't see a clear winner.

I think probably the only real world use case where it might really be helpful is if you wanted to accept a custom type as a field input and had a custom __bool__ implementation.

At any rate, based on the above I can't think of anything worth saying in the docs except that it follows built-in python logic for boolean casting (what they currently say). Let me know if you still disagree, or if this changes your mind back to wanting to remove RelaxedBool.

I would say let's remove it then. Nothing to stop people implementing it themselves if they really want it. But we we think there's basically no context in which you should use it, it shouldn't be added.

Sorry to flip flop.

pydantic/validators.py

tests/test_types.py

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

Co-authored-by: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Co-authored-by: Samuel Colvin <s@muelcolvin.com> Co-authored-by: David Montague <35119617+dmontagu@users.noreply.github.com>

dmontagu commented Jun 23, 2019

View reviewed changes

pydantic/validators.py Outdated Show resolved Hide resolved

dmontagu commented Jun 23, 2019

View reviewed changes

tests/test_types.py Show resolved Hide resolved

samuelcolvin reviewed Jun 26, 2019

View reviewed changes

pydantic/validators.py Outdated Show resolved Hide resolved

cazgp reviewed Jun 26, 2019

View reviewed changes

pydantic/validators.py Outdated Show resolved Hide resolved

dmontagu force-pushed the strictbool branch from 3988844 to 4a46f2f Compare June 27, 2019 09:09

dmontagu commented Jun 27, 2019

View reviewed changes

tests/test_types.py Show resolved Hide resolved

samuelcolvin reviewed Jun 28, 2019

View reviewed changes

samuelcolvin mentioned this pull request Jun 28, 2019

Version 1 Release #576

Closed

5 tasks

dmontagu force-pushed the strictbool branch 3 times, most recently from 7dc18d0 to e0bfe47 Compare July 16, 2019 04:45

dmontagu force-pushed the strictbool branch 2 times, most recently from 21c28d8 to 773e160 Compare July 25, 2019 07:19

samuelcolvin mentioned this pull request Jul 26, 2019

How to provide default value to StrictBool? #690

Closed

samuelcolvin added this to the Version 1 milestone Aug 5, 2019

dmontagu force-pushed the strictbool branch from 773e160 to 46697bb Compare August 7, 2019 10:58

samuelcolvin reviewed Aug 8, 2019

View reviewed changes

David Montague added 7 commits August 8, 2019 04:22

Make bool_validator strict

d0a2af3

incorporate feedback

deca137

Add RelaxedBool

2fbcf98

Fix position in schema.py

afa78e8

update BoolError message

d945b74

Incorporate feedback

c986d7a

Update history

2dda2ba

David Montague and others added 4 commits August 8, 2019 04:22

Add changes

fbd2d59

Update docs/index.rst

360bc0e

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

Update docs/index.rst

2613e7f

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

Update tests/test_types.py

ff157e9

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>

dmontagu force-pushed the strictbool branch from eab3ffc to ecd6ec1 Compare August 8, 2019 11:31

David Montague and others added 4 commits August 8, 2019 04:32

Incorporate feedback

ecd6ec1

Remove RelaxedBool

0fb8744

Update booleans.py

34e48af

tweak docs and update changes to new format

c5b6f0d

samuelcolvin merged commit 72edca7 into pydantic:master Aug 10, 2019

samuelcolvin mentioned this pull request Aug 12, 2019

StrictBool #579

Closed

samuelcolvin mentioned this pull request Aug 27, 2019

Generic method to create Strict<type> fields #780

Closed

samuelcolvin mentioned this pull request Sep 9, 2019

Type Validation with Dataclasses and Marshmallow #797

Closed

	for some values: for example, a ``RelaxedBool`` field would cast the string "false" to True.
	for some values: for example, a ``RelaxedBool`` field would cast the string ``"false"`` to ``True``.

		@@ -243,6 +244,21 @@ def validate(cls, value: str) -> str:
		return validate_email(value)[1]


		class RelaxedBool(int):

Uh oh!

Make bool_validator strict #617

Make bool_validator strict #617

Uh oh!

Conversation

dmontagu commented Jun 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Summary

Related issue number

Checklist

Uh oh!

Uh oh!

codecov bot commented Jun 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cazgp commented Jun 26, 2019

Uh oh!

Uh oh!

dmontagu commented Jun 28, 2019

Uh oh!

lgtm-com bot commented Jun 28, 2019

Uh oh!

lgtm-com bot commented Jun 28, 2019

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Jun 28, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samuelcolvin commented Aug 6, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

samuelcolvin Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

dmontagu Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

dmontagu Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Aug 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmontagu commented Jun 23, 2019 •

edited

Loading

codecov bot commented Jun 23, 2019 •

edited

Loading

dmontagu Aug 8, 2019 •

edited

Loading

dmontagu Aug 8, 2019 •

edited

Loading