Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix schema for timedelta as number, instead of str #325

Merged
merged 1 commit into from Dec 23, 2018

Conversation

Projects
None yet
2 participants
@tiangolo
Copy link
Contributor

tiangolo commented Dec 22, 2018

Change Summary

Currently, the generated schema for datetime.timedelta is as string.

But by default, Pydantic will convert it to a float with the difference in seconds.

(Bug created by me in the JSON Schema compatibility PR).

This PR fixes that and makes the schema for timedelta be a number by default, which corresponds to the actual behavior.

Related issue number

I didn't create one.

Performance Changes

pydantic cares about performance, if there's any risk performance changed on this PR,
please run make benchmark-pydantic before and after the change:

  • before: ?
  • after: ?

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes
  • No performance deterioration (if 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>
    • if you're not a regular contributer please include your github username @whatever
@codecov

This comment has been minimized.

Copy link

codecov bot commented Dec 22, 2018

Codecov Report

Merging #325 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master   #325   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          13     13           
  Lines        1771   1771           
  Branches      344    344           
=====================================
  Hits         1771   1771

@samuelcolvin samuelcolvin merged commit 81f1558 into samuelcolvin:master Dec 23, 2018

3 checks passed

codecov/project 100% remains the same compared to 1f270dd
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@samuelcolvin

This comment has been minimized.

Copy link
Owner

samuelcolvin commented Dec 23, 2018

great, thanks. will release in a couple of days.

@tiangolo

This comment has been minimized.

Copy link
Contributor Author

tiangolo commented Dec 23, 2018

Awesome, thanks. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.