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

Added generic approach to strict type checking for constrained types #799

Merged
merged 14 commits into from
Sep 17, 2019

Conversation

DerRidda
Copy link
Contributor

@DerRidda DerRidda commented Sep 9, 2019

Change Summary

  • Use the arbitrary validator to build strict validators for ConstrainedInt, ConstrainedFloat, ConstrainedStr
  • Make StrictStr a derived class of ConstrainedStr
  • Add tests for new strict cases for ConstrainedInt and ConstrainedFloat

This PR adds configurable strictness validation to ConstrainedStr, ConstrainedInt and ConstrainedFloat.

This behaviour is desired for our use case as we generally never want coercion for anything,
especially those that loses information such as float -> int coercion.

Related issue number

This issue seems related but was not regarded when creating this PR: #780

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.rst file added describing change
    (see changes/README.md for details)

- Use the arbitrary validator to build strict validators for ConstrainedInt, ConstrainedFloat, ConstrainedStr
- Make StrictStr a derived class of ConstrainedStr
- Add tests for new strict cases for ConstrainedInt and ConstrainedFloat
@codecov
Copy link

codecov bot commented Sep 9, 2019

Codecov Report

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

@@          Coverage Diff          @@
##           master   #799   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          17     17           
  Lines        2972   2984   +12     
  Branches      578    580    +2     
=====================================
+ Hits         2972   2984   +12
Impacted Files Coverage Δ
pydantic/types.py 100% <100%> (ø) ⬆️
pydantic/validators.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0c57937...359b468. Read the comment docs.

@samuelcolvin
Copy link
Member

I think this looks good.

Please can you

  • also export StrictFloat and StrictInt (same way you do StrictStr)
  • add something to the docs about this, perhaps by extending and changing the StrictBool section
  • add a change description.

@DerRidda
Copy link
Contributor Author

DerRidda commented Sep 9, 2019

I have added all requested changes.

Docs have their own section now.

@DerRidda DerRidda changed the title Added generic approach to strict type checking for constraint types Added generic approach to strict type checking for constrained types Sep 9, 2019
docs/index.rst Outdated Show resolved Hide resolved
DerRidda and others added 2 commits September 11, 2019 10:08
Co-Authored-By: Zaar Hai <haizaar@users.noreply.github.com>
Co-Authored-By: Zaar Hai <haizaar@users.noreply.github.com>
@@ -785,6 +785,14 @@ The SecretStr and SecretBytes will be formatted as either `'**********'` or `''`

(This script is complete, it should run "as is")

Strict Types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move the "StrictBool" section into here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to remove the "StrictBool" section: https://pydantic-docs.helpmanual.io/#strictbool

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

docs/index.rst Outdated Show resolved Hide resolved
These types will only pass validation when the validated value is of the respective type or is a subtype of that type.
This behavior is also exposed via the ``strict`` field of the ``ConstrainedStr``, ``ConstrainedFloat`` and
``ConstrainedInt`` classes and can be combined with a multitude of complex validation rules (please note that there is no ``ConstrainedBool`` class).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to add a code example demonstrating this behaviour.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

pydantic/types.py Outdated Show resolved Hide resolved
tests/test_types.py Outdated Show resolved Hide resolved
tests/test_types.py Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
- Make ConstrainedInt and ConstrainedFloat use those validators instead of abusing arbitrary type validator for strictness
- Prevent double validaton of same conditions by only yielding either the strict or non-strict type validator for for those classes
- Added example for strict type usage
- Added note about caveats for StrictInt and StrictFloat
pydantic/validators.py Outdated Show resolved Hide resolved
@@ -785,6 +785,14 @@ The SecretStr and SecretBytes will be formatted as either `'**********'` or `''`

(This script is complete, it should run "as is")

Strict Types
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still need to remove the "StrictBool" section: https://pydantic-docs.helpmanual.io/#strictbool

pydantic/validators.py Show resolved Hide resolved
DerRidda and others added 4 commits September 16, 2019 09:55
faster method to check if a value is boolean for strict int validator

Co-Authored-By: Samuel Colvin <samcolvin@gmail.com>
- Moved the Strctbool code example into strict_types.py example file
@@ -617,12 +617,9 @@ A standard ``bool`` field will raise a ``ValidationError`` if the value is not o
``'off', 'f', 'false', 'n', 'no', '1', 'on', 't', 'true', 'y', 'yes'``
* a ``bytes`` which is valid (per the previous rule) when decoded to ``str``

For stricter behavior, ``StrictBool`` can be used to require specifically ``True`` or ``False``;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we keep this line and add a link to the "Strict Types" section.

Sorry to mess you around.

pydantic/validators.py Show resolved Hide resolved

@classmethod
def __get_validators__(cls) -> 'CallableGenerator':
yield not_none_validator
yield str_validator
yield make_arbitrary_type_validator(str) if cls.strict else str_validator
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this work if you pass it an enum that inherits from str? Perhaps it should, but it will likely confuse many people since the __str__ method is different from normal I think.

We should at least add a note to the docs.

Copy link
Contributor Author

@DerRidda DerRidda Sep 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naturally this should pass with all subclasses as we only make the exception for integers with boolean as that is a weird quirk of Python where Guido probably looked at C too much.

I can add the note to the docs.

@samuelcolvin samuelcolvin merged commit 1b467da into pydantic:master Sep 17, 2019
@samuelcolvin
Copy link
Member

great, thank you so much.

@DerRidda
Copy link
Contributor Author

Thank you for merging, I was actually still touching on a few things you had commented on but if it is fine like that I don't mind.

@samuelcolvin
Copy link
Member

sorry about that, I'm crashing about trying to get to v1. Feel free to submit another PR if you have any tweaks.

@skewty
Copy link
Contributor

skewty commented Nov 18, 2019

I deleted my previous posts because the solution I came up with in the end seems trivial.

I guess responses were created due to emails generated.. So here is a response to a question that disappeared :)

@dmontagu

Why can't you do IdentStr = typing.NewType('IdentStr', ConstrainedStr) or similar?

import pydantic, typing

IdentStr = typing.NewType('IdentStr', pydantic.ConstrainedStr)
IdentStr.min_length

Traceback (most recent call last):
  File "<input>", line 1, in <module>
AttributeError: 'function' object has no attribute 'min_length'

typing.NewType only fakes making a real class.


Since in real life, we control the code for IdentStr, I am playing around with:

if PYDANTIC_VERSION >= "1.0.0":
    from pydantic import StrictStr
else:  # min version is 0.32.2 in setup.py
    from pydantic import ConstrainedStr
    from pydantic.errors import StrError

    class StrictStr(ConstrainedStr):
        @classmethod
        def strict_str_validator(cls, v: str) -> str:
            if not isinstance(v, str):
                raise StrError()
            return v

        @classmethod
        def __get_validators__(cls):
            yield cls.strict_str_validator
            yield from super().__get_validators__()

class IdentStr(StrictStr):
    min_length = 1

@DerRidda DerRidda deleted the strict-constrained-types branch November 18, 2019 09:09
@skewty
Copy link
Contributor

skewty commented Dec 1, 2019

I am sorta lost as to why this isn't working as expected for me in v1.2:

from typing import Type, TypeVar
import pydantic


class ConnectionName(pydantic.StrictStr):
    min_length = 1


class EndpointStr(pydantic.StrictStr):
    min_length = 1


DEFAULT_ENDPOINT = EndpointStr("default")
T = TypeVar("T", bound="UpRequestEvent")


class UpRequestEvent(pydantic.BaseModel):
    connection_name: ConnectionName  # connection name
    connection_endpoint: EndpointStr  # connection name's endpoint (for server type connections)

    @classmethod
    def build_from(
        cls: Type[T],
        connection_name: ConnectionName,
        connection_endpoint: EndpointStr = DEFAULT_ENDPOINT,
        **kwargs
    ) -> T:
        return cls(
            connection_name=connection_name,
            connection_endpoint=connection_endpoint,
        )

    class Config:
        use_enum_values = True


# This works
UpRequestEvent.build_from(
    connection_name="test", connection_endpoint="default",
)

# This fails
UpRequestEvent.build_from(
    connection_name=ConnectionName("test"), connection_endpoint=EndpointStr("default"),
)
Traceback (most recent call last):
  File "/opt/pycharm-eap/plugins/python/helpers/pydev/pydevd.py", line 1415, in _exec
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/opt/pycharm-eap/plugins/python/helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/home/spalmer1009/.PyCharm2019.3/config/scratches/scratch.py", line 44, in <module>
    connection_name=ConnectionName("test"), connection_endpoint=EndpointStr("default"),
  File "/home/spalmer1009/.PyCharm2019.3/config/scratches/scratch.py", line 30, in build_from
    connection_endpoint=connection_endpoint,
  File "pydantic/main.py", line 274, in pydantic.main.BaseModel.__init__
pydantic.error_wrappers.ValidationError: 2 validation errors for UpRequestEvent
connection_name
  Expected unicode, got ConnectionName (type=type_error)
connection_endpoint
  Expected unicode, got EndpointStr (type=type_error)

My special types ConnectionName and EndpointStr are a subclass of str (through StrictStr -> ConstrainedStr)

Am I not able to subclass these two types?

@dmontagu
Copy link
Contributor

dmontagu commented Dec 1, 2019

This appears to be an issue with the compiled vs. uncompiled versions of the package, as it works for me when not compiled (and fails when compiled). I'm trying to figure out if there is a workaround/solution now.

@dmontagu
Copy link
Contributor

dmontagu commented Dec 1, 2019

@skewty I think I found the issue.

If you change the signature of ConstrainedStr.validate to

def validate(cls, value: Union[str]) -> Union[str]: ...

and of strict_str_validator to:

def strict_str_validator(v: Any) -> Union[str]:

your code works.

I think what is happening is something weird with how cython handles str; using Union[str] prevents cython from applying some compilation magic.

@skewty It would be great if you could submit a new (bug) issue about this. It would also be great if you could make a PR with these changes and adding an appropriate test, but we can get to it eventually if you create an issue.

@dmontagu
Copy link
Contributor

dmontagu commented Dec 1, 2019

Strangely, looking at the differences in the generated cython code, you can see cython is actually injecting this error:

+060:         return v

    __Pyx_XDECREF(__pyx_r);
    if (!(likely(PyString_CheckExact(__pyx_v_v))||((__pyx_v_v) == Py_None)||(PyErr_Format(PyExc_TypeError, "Expected %.16s, got %.200s", "str", Py_TYPE(__pyx_v_v)->tp_name), 0))) __PYX_ERR(0, 60, __pyx_L1_error)
    __Pyx_INCREF(__pyx_v_v);
    __pyx_r = ((PyObject*)__pyx_v_v);

(In the -> Union[str] version no type error is injected like this.)

It also makes another minor change in the code earlier on, but I can't tell if that is affecting anything meaningful at first glance (looks like it is just looking up the Union type).

alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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.

5 participants