fix type annotations for exotic types; re-use type validators in exotic types

[Gr1N]

Changes:

• fixed type annotations for exotic types due to new release 0.600 of mypy
• also I tried to simplify codebase and re-use type validators in exotic types, less code better to my mind, please let me know what do you think and what I need to improve

To discuss:

• I would like to rename gt and lt in ConstrainedInt and ConstrainedFloat because we already have min_number_size and max_number_size for int and float and to my mind it looks like kind of inconsistency. And I think we can do this backward compatible by adding new min_size and max_size without removing gt and lt. What do you think?

[codecov]

Codecov Report

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

@@          Coverage Diff          @@
##           master   #171   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         950    939   -11     
  Branches      207    200    -7     
=====================================
- Hits          950    939   -11

[Gr1N]

@samuelcolvin ping! :)

[samuelcolvin]

Otherwise looking good.

Please can you run benchmarks (python benchmarks/run.py pydantic-only) for master and this branch and include the results here - we should just check that this hasn't caused a significant performance impact.

[samuelcolvin]

Surely cleaner here to use Union[None, int, float]

[samuelcolvin]

I guess here you can use

if cls.curtail_length and len(value) > cls.curtail_length:
    ...

Since the v_len variable isn't reused.

[Gr1N]

Benchmarks on master branch:

$ python benchmarks/run.py pydantic-only
generating test cases...
                                pydantic time=0.651s, success=48.30%
                                pydantic time=0.688s, success=48.30%
                                pydantic time=0.667s, success=48.30%
                                pydantic time=0.658s, success=48.30%
                                pydantic time=0.664s, success=48.30%
                                pydantic best=0.651s, avg=0.666s, stdev=0.014s

                                pydantic best=21.707μs/iter avg=22.188μs/iter stdev=0.467μs/iter

And for this branch:

$ python benchmarks/run.py pydantic-only
                                pydantic time=0.664s, success=48.30%
                                pydantic time=0.670s, success=48.30%
                                pydantic time=0.677s, success=48.30%
                                pydantic time=0.650s, success=48.30%
                                pydantic time=0.661s, success=48.30%
                                pydantic best=0.650s, avg=0.665s, stdev=0.010s

                                pydantic best=21.678μs/iter avg=22.156μs/iter stdev=0.336μs/iter

[samuelcolvin]

Thanks.

In future please don't amend commits, I squash merge anyway and amending commits makes it harder to see the changes after my comments.

[Gr1N]

In future please don't amend commits, I squash merge anyway and amending commits makes it harder to see the changes after my comments.

No problem

@samuelcolvin what do you think about?

I would like to rename gt and lt in ConstrainedInt and ConstrainedFloat because we already have min_number_size and max_number_size for int and float and to my mind it looks like kind of inconsistency. And I think we can do this backward compatible by adding new min_size and max_size without removing gt and lt. What do you think?

[samuelcolvin]

I would like to rename gt and lt in ConstrainedInt and ConstrainedFloat because we already have min_number_size and max_number_size for int and float and to my mind it looks like kind of inconsistency. And I think we can do this backward compatible by adding new min_size and max_size without removing gt and lt. What do you think?

Sounds good to me. I would do it so usage of lt an gt raises a helpful exception.

We also need to be careful about the exact meaning min_number_size is kind of the same as gte but not the same as gt.

Eg. with gt=1 would indicate that 1 is not an allowed value, but min_number_size=1 would indicate that 1 is an allowed value.

I think the second case makes more sense but most importantly we should be consistent and explicit.

[Gr1N]

So to be clear. As I understand you're good with proposal and we can replace gt and lt with min_size and max_size, but I want to understand right way to implement those changes.

In code base we already have an issue with gt and lt because we're using >= and <= instead of > and < (https://github.com/samuelcolvin/pydantic/blob/v0.9/pydantic/types.py#L187) and I think we need to fix it.

I see there ways:

1. Backward incompatible changes: just rename to min_size and max_size.
2. Backward compatible changes: keep lt and gt, but on code level we will go to convert them to min_size and max_size and write deprecation warning to stdout using warnings package.
3. Same as 2, but raise an exception in runtime if lt and gt will be used.

From code-level I prefer option 1 (I don't like warnings and less code better), but from community side I think option 2 is better.

What do you think? Maybe I don't understand you and you have better solution?

[samuelcolvin]

Code is currently correct for the names lt and gt, I think. But will need to change to match the new names.

Hence why we need a hard depication lt and gt, because the meaning will change.

Shall I do a pr and you can review?

[Gr1N]

Shall I do a pr and you can review?

Yep!