Skip to content

Change range/float range to value error and include member/obj in error message#221

Merged
MatthieuDartiailh merged 4 commits intonucleic:mainfrom
frmdstryr:update-range-value-error-messages
Jan 20, 2025
Merged

Change range/float range to value error and include member/obj in error message#221
MatthieuDartiailh merged 4 commits intonucleic:mainfrom
frmdstryr:update-range-value-error-messages

Conversation

@frmdstryr
Copy link
Contributor

Currently a value out of range throws a type error even if it is the correct type.

This also includes the member name in the error message.

@frmdstryr
Copy link
Contributor Author

Well it seems something changed in the float_range_handler with 3.12.

I started getting 'too small' validation errors with FloatRange(low=0, strict=False) for a value of 25. After updating the message to print the low value it was some huge value?

@frmdstryr
Copy link
Contributor Author

If I change low value to a float low=0.0 it works it seems the low & high types are not checked.

@MatthieuDartiailh
Copy link
Member

Can you rebase on main to fix the CI issue. Also it may be worth casting the low, high to preserve the fast validation path.

@frmdstryr frmdstryr force-pushed the update-range-value-error-messages branch from 2a6fa7e to db719dc Compare January 11, 2025 14:03
@frmdstryr frmdstryr force-pushed the update-range-value-error-messages branch from db719dc to e8ff667 Compare January 11, 2025 14:05
@frmdstryr frmdstryr force-pushed the update-range-value-error-messages branch from e8ff667 to aa1dd73 Compare January 11, 2025 14:07
@frmdstryr frmdstryr force-pushed the update-range-value-error-messages branch from b69a936 to 699e66f Compare January 11, 2025 14:27
@frmdstryr
Copy link
Contributor Author

The problem was there was no context validation for FloatRangePromote since the case was missing.

@codecov
Copy link

codecov bot commented Jan 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.67%. Comparing base (94baafc) to head (f57b23d).
Report is 15 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #221      +/-   ##
==========================================
+ Coverage   97.27%   97.67%   +0.39%     
==========================================
  Files          24       24              
  Lines        1065     1074       +9     
  Branches      158      162       +4     
==========================================
+ Hits         1036     1049      +13     
+ Misses         14       12       -2     
+ Partials       15       13       -2     

@MatthieuDartiailh MatthieuDartiailh merged commit 23aaa52 into nucleic:main Jan 20, 2025
18 checks passed
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.

2 participants