Add eitherfloat #691

JeanArhancet · 2023-06-23T12:59:37Z

Change Summary

Add Eitherfloat

Related issue number

fix #639

Checklist

Unit tests for the changes exist
Documentation reflects the changes where applicable
Pydantic tests pass with this pydantic-core (except for expected changes)
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

Selected Reviewer: @dmontagu

JeanArhancet · 2023-06-23T13:21:39Z

please review

codspeed-hq · 2023-06-23T13:27:21Z

CodSpeed Performance Report

Merging #691 JeanArhancet:feat/use-eitherfloat (092be01) will improve performances by 11.94%.

Summary

🔥 3 improvements
❌ 0 regressions
✅ 122 untouched benchmarks

🆕 0 new benchmarks
⁉️ 0 dropped benchmarks

Benchmarks breakdown

	Benchmark	`main`	`JeanArhancet:feat/use-eitherfloat`	Change
🔥	`test_complete_core_error`	9.6 ms	8.6 ms	+11.94%
🔥	`test_complete_core_isinstance`	9.6 ms	8.6 ms	+11.74%
🔥	`test_set_of_ints_core_json_duplicates`	5.8 ms	5.2 ms	+10.66%

codecov-commenter · 2023-06-23T13:35:10Z

Codecov Report

Merging #691 (092be01) into main (875befe) will decrease coverage by 0.03%.
The diff coverage is 84.78%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #691      +/-   ##
==========================================
- Coverage   93.72%   93.69%   -0.03%     
==========================================
  Files          99       99              
  Lines       13802    13822      +20     
  Branches       25       25              
==========================================
+ Hits        12936    12951      +15     
- Misses        860      865       +5     
  Partials        6        6

Impacted Files	Coverage Δ
src/input/mod.rs	`100.00% <ø> (ø)`
src/input/return_enums.rs	`85.00% <50.00%> (-0.68%)`	⬇️
src/input/input_json.rs	`90.07% <86.66%> (ø)`
src/errors/types.rs	`83.74% <100.00%> (ø)`
src/input/input_abstract.rs	`86.11% <100.00%> (ø)`
src/input/input_python.rs	`98.09% <100.00%> (+0.03%)`	⬆️
src/validators/float.rs	`90.37% <100.00%> (+0.07%)`	⬆️

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

src/errors/types.rs

tests/test_errors.py

src/errors/types.rs

dmontagu

@adriangb or @samuelcolvin I think one of you should look at this before we merge but looks okay to me.

Also, we'll need to update the pydantic docs in a separate PR to cover this new error code.

Co-authored-by: David Montague <[email protected]>

samuelcolvin

otherwise looking good.

Please update.

samuelcolvin · 2023-06-26T13:54:55Z

pydantic_core/core_schema.py

@@ -3802,6 +3802,7 @@ def definition_reference_schema(
    'int_from_float',
    'float_type',
    'float_parsing',
+    'float_parsing_size',


do we have a test which actually raises this error?

If it's not possible to actually made it happen, or very rare, I think we just use float_parsing.

I didn't manage to test it. I replaced it with float_parsing

src/errors/types.rs

samuelcolvin · 2023-06-26T13:56:51Z

src/input/input_python.rs

@@ -302,35 +302,40 @@ impl<'a> Input<'a> for PyAny {
        }
    }

-    fn ultra_strict_float(&self) -> ValResult<f64> {
+    fn ultra_strict_float(&'a self) -> ValResult<EitherFloat<'a>> {
        if self.is_instance_of::<PyInt>() {
            Err(ValError::new(ErrorType::FloatType, self))
        } else if let Ok(float) = self.extract::<f64>() {


we shouldn't use extract here, but rather is_instance_of, then use EitherFloat::Py.

Done, thanks

samuelcolvin · 2023-06-26T13:58:17Z

src/input/input_python.rs

@@ -302,35 +302,40 @@ impl<'a> Input<'a> for PyAny {
        }
    }

-    fn ultra_strict_float(&self) -> ValResult<f64> {
+    fn ultra_strict_float(&'a self) -> ValResult<EitherFloat<'a>> {


I think with this change we might be able to remove ultra_strict_float completely, maybe that's too much work for this PR.

If I remove the functionultra_strict_float, do I have to change the behavior of strict_float and lax_float?

leave it for now.

JeanArhancet · 2023-06-26T14:43:22Z

please review

feat: add use eitherfloat

8fc0231

JeanArhancet force-pushed the feat/use-eitherfloat branch 2 times, most recently from 85cdccc to cf833be Compare June 23, 2023 13:06

feat: use eitherfloat

ca38362

JeanArhancet force-pushed the feat/use-eitherfloat branch from cf833be to ca38362 Compare June 23, 2023 13:07

Merge branch 'main' into feat/use-eitherfloat

8ee844e

pydantic-hooky bot added the ready for review label Jun 23, 2023

pydantic-hooky bot assigned dmontagu Jun 23, 2023

fix: unit test

1ad6184

raph-af reviewed Jun 23, 2023

View reviewed changes

src/errors/types.rs Outdated Show resolved Hide resolved

dmontagu reviewed Jun 23, 2023

View reviewed changes

tests/test_errors.py Outdated Show resolved Hide resolved

dmontagu reviewed Jun 23, 2023

View reviewed changes

tests/test_errors.py Outdated Show resolved Hide resolved

dmontagu reviewed Jun 23, 2023

View reviewed changes

src/errors/types.rs Outdated Show resolved Hide resolved

dmontagu approved these changes Jun 23, 2023

View reviewed changes

JeanArhancet and others added 2 commits June 24, 2023 09:28

Apply suggestions from code review

ceec85c

Co-authored-by: David Montague <[email protected]>

fix: typo in test

3180d8d

JeanArhancet force-pushed the feat/use-eitherfloat branch from 565a9db to 3180d8d Compare June 24, 2023 07:41

samuelcolvin reviewed Jun 26, 2023

View reviewed changes

pydantic-hooky bot added awaiting author revision and removed ready for review labels Jun 26, 2023

pydantic-hooky bot assigned JeanArhancet and unassigned dmontagu Jun 26, 2023

fix: apply feedback

092be01

pydantic-hooky bot added the ready for review label Jun 26, 2023

pydantic-hooky bot removed the awaiting author revision label Jun 26, 2023

pydantic-hooky bot assigned dmontagu and unassigned JeanArhancet Jun 26, 2023

samuelcolvin merged commit a55bd2e into pydantic:main Jun 28, 2023

davidhewitt added a commit that referenced this pull request Jun 28, 2023

fix clippy failure from #691 post-merge

1bd9198

davidhewitt mentioned this pull request Jun 28, 2023

fix stubtest issue post-merge #711

Merged

4 tasks

Add eitherfloat #691

Add eitherfloat #691

Uh oh!

Conversation

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

Uh oh!

Change Summary

Related issue number

Checklist

Uh oh!

JeanArhancet commented Jun 23, 2023

Uh oh!

codspeed-hq bot commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Benchmarks breakdown

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dmontagu left a comment

Choose a reason for hiding this comment

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JeanArhancet commented Jun 26, 2023

Uh oh!

Uh oh!

JeanArhancet commented Jun 23, 2023 •

edited

Loading

codspeed-hq bot commented Jun 23, 2023 •

edited

Loading

codecov-commenter commented Jun 23, 2023 •

edited

Loading