Skip to content

Add better support for floating point multiple_of values#652

Merged
samuelcolvin merged 1 commit intopydantic:masterfrom
justindujardin:bugfix/float_multiple_of
Jul 23, 2019
Merged

Add better support for floating point multiple_of values#652
samuelcolvin merged 1 commit intopydantic:masterfrom
justindujardin:bugfix/float_multiple_of

Conversation

@justindujardin
Copy link
Copy Markdown
Contributor

@justindujardin justindujardin commented Jul 10, 2019 •
edited
Loading

Change Summary

Add support for multiple_of with arbitrary int/floating point numbers. The current implementation fails for obvious floating point values, e.g. 0.3 fails to validate as a multiple of 0.1.

Related issue number

None

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • HISTORY.rst has been updated
    • if this is the first change since a release, please add a new section
    • include the issue number or this pull request number #<number>
    • include your github username @<whomever>

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 10, 2019 •
edited
Loading

Codecov Report

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

@@          Coverage Diff          @@
##           master   #652   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2651   2655    +4     
  Branches      524    526    +2     
=====================================
+ Hits         2651   2655    +4

@justindujardin justindujardin changed the title 🐛 fix for floating point multiple_of values Add better support for floating point multiple_of values Jul 10, 2019
samuelcolvin
samuelcolvin reviewed Jul 11, 2019
View reviewed changes
Comment thread pydantic/validators.py
Comment thread tests/test_types.py Outdated
@justindujardin
Copy link
Copy Markdown
Contributor Author

justindujardin commented Jul 11, 2019

While working on this I noticed that JSONSchema spec says mutliple_of must strictly be greater than 0.0, so I added a validation error and a test for this behavior.

@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Jul 11, 2019

You're checking v not multiple_of.

Also your calling float twice.

@justindujardin
Copy link
Copy Markdown
Contributor Author

justindujardin commented Jul 11, 2019

Oof, yeah. Is there a more appropriate place to check this then? Somewhere maybe it could be an error during declaration rather than at validation time?

I reverted the extra validation commit for the time being.

@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Jul 11, 2019

(on my mobile)

Yes, they're better places, probably in validators.py.

@justindujardin
Copy link
Copy Markdown
Contributor Author

justindujardin commented Jul 13, 2019

@samuelcolvin FYI I'm not planning on following up with the > 0 assertion in this PR. If there's anything else you would like changed to support floating point multiple_of let me know. Thanks for your review!

tiangolo
tiangolo approved these changes Jul 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@samuelcolvin
Copy link
Copy Markdown
Member

samuelcolvin commented Jul 15, 2019

LGTM, @justindujardin please could you rebase and move the new history line to the new section.

I'm aware the history thing is getting annoying I'll try to adopt towncrier or similar soon.

@justindujardin
Add better support for floating point multiple_of values
de88ddf 
 - modulo doesn't work with floating point values in many cases, e.g. `0.3 % 0.1 == 0.09999999999999998`
 - port implementation from: tdegrunt/jsonschema#187 (comment)
 - add tests for int/float multiple_of values
 - update history with pr/author
@justindujardin
Copy link
Copy Markdown
Contributor Author

justindujardin commented Jul 15, 2019

@samuelcolvin sure thing, done

@samuelcolvin samuelcolvin merged commit bc60014 into pydantic:master Jul 23, 2019
@justindujardin justindujardin deleted the bugfix/float_multiple_of branch July 23, 2019 18:50
alexdrydew pushed a commit to alexdrydew/pydantic that referenced this pull request Dec 23, 2023
@adriangb
Move dataclass config onto dataclass schema (pydantic#652)
ca48482
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@samuelcolvin samuelcolvin samuelcolvin left review comments

+1 more reviewer

@tiangolo tiangolo tiangolo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justindujardin @samuelcolvin @tiangolo