avoid coercing int subclasses to floats

[samuelcolvin]

Change Summary

Fix validation if int subclasses.

With this we now upcast sub-types of int to int in both strict and lax mode, the rationale is as follows:

• the behaviour before was to upcast (via the extract::<f64>() logic which we're now avoiding)
• hence we're keeping the same behaviour in the very common case of lax ints - we even had tests for that in pydantic
• we now do the same in "strict" mode to match lax mode

Related issue number

fix pydantic/pydantic#7201

fix #913

Checklist

• Unit tests for the changes exist
• 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: @adriangb

[codspeed-hq]

CodSpeed Performance Report

Merging #914 will improve performances by 12.32%

_{Comparing int-subclasses (8e309d3) with main (7f7d03d)}

Summary

🔥 2 improvements
✅ 136 untouched benchmarks

Benchmarks breakdown

	Benchmark	main	int-subclasses	Change
🔥	test_core_future_str	35.9 µs	32 µs	+12.12%
🔥	test_validate_literal[json-few_str_enum]	38 µs	33.9 µs	+12.32%

[codecov]

Codecov Report

Merging #914 (8e309d3) into main (7f7d03d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #914   +/-   ##
=======================================
  Coverage   93.78%   93.79%           
=======================================
  Files         105      105           
  Lines       15331    15335    +4     
  Branches       25       25           
=======================================
+ Hits        14378    14383    +5     
+ Misses        947      946    -1     
  Partials        6        6           

Files Changed	Coverage Δ
src/input/input_python.rs	98.30% <100.00%> (+0.26%)	⬆️

---

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 7f7d03d...8e309d3. Read the comment docs.

[samuelcolvin]

please review.

[samuelcolvin]

we now do the same in "strict" mode to match lax mode

i would definitely consider reverting this change - substituting consistency between lax and strict for a smaller change of breaking changes.

[davidhewitt]

Personally I think we want to go the other way and not upcast in both, I already can't remember why we wanted the upcast in lax mode.

Regardless of to upcast or not upcast, I think both modes should behave the same, so we should change one or the other.

Also you might want to consider using BigInt here if the i64 extraction fails.

[adriangb]

Just for my understanding, this means that now if isinstance(obj, int) is true then we essentially do self.extract::<i64>() which upcasts into an exact int type eventually. Previously subclasses of ints would fail to validate as an int and somehow end up as a float?

[adriangb]

I pulled your linting changes into #916 to avoid conflict with #902

[samuelcolvin]

Just for my understanding, this means that now if isinstance(obj, int) is true then we essentially do self.extract::<i64>() which upcasts into an exact int type eventually. Previously subclasses of ints would fail to validate as an int and somehow end up as a float?

well, end up being coerced to a float, then back to an int - hence with large numbers precision was lost. Otherwise yes.

[samuelcolvin]

@roman-right are you happy with this - particularly the treatment of subclasses?

[NiceAesth]

Also related: #913

[roman-right]

@roman-right are you happy with this - particularly the treatment of subclasses?

Hi @samuelcolvin ! It works well for my case. Thank you.