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

timedelta json encoding #247

Merged
merged 7 commits into from Aug 25, 2018

Conversation

2 participants
@samuelcolvin
Owner

samuelcolvin commented Aug 20, 2018

an alternative to #220. Needs a few more tests and docs but otherwise complete.

basically, I've added a json_encoders Config settings, allowing custom typing encoding defined on a model:

class Model(BaseModel):
    x: datetime.timedelta

    class Config:
        json_encoders = {datetime.timedelta: timedelta_isoformat}

m = Model(x=123)
assert m.json() == '{"x": "P0DT0H2M3.000000S"}'

@Gr1N and @cfkanesan I'd appreciate your feedback on this.

@codecov

This comment has been minimized.

codecov bot commented Aug 20, 2018

Codecov Report

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

@@          Coverage Diff          @@
##           master   #247   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          11     11           
  Lines        1463   1478   +15     
  Branches      270    273    +3     
=====================================
+ Hits         1463   1478   +15
@@ -134,11 +136,16 @@ def __new__(mcs, name, bases, namespace):
)
vg.check_for_unused()
if config.json_encoders:

This comment has been minimized.

@Gr1N

Gr1N Aug 20, 2018

Collaborator

We can simplify 4 lines into one, because pydantic_encoder will be called inside custom_pydantic_encoder in case if custom encoder not found:

json_encoder = partial(custom_pydantic_encoder, config.json_encoders)

This comment has been minimized.

@samuelcolvin

samuelcolvin Aug 20, 2018

Owner

Are we sure this won't be slow? I don't really know.

This comment has been minimized.

@Gr1N

Gr1N Aug 20, 2018

Collaborator

It can be slow, agree

This comment has been minimized.

@samuelcolvin

samuelcolvin Aug 20, 2018

Owner

Given that it's not much extra code and json_encoder could be called a lot, let's keep it like this.

return pydantic_encoder(obj)
def timedelta_isoformat(td: datetime.timedelta):

This comment has been minimized.

@Gr1N

Gr1N Aug 20, 2018

Collaborator

missing returning type hint

@Gr1N

This comment has been minimized.

Collaborator

Gr1N commented Aug 20, 2018

Overall looks good to me.

@samuelcolvin samuelcolvin merged commit f46dc0c into master Aug 25, 2018

5 checks passed

codecov/project 100% (+0%) compared to e0a8dd2
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
pyup.io/safety-ci No dependencies with known security vulnerabilities.
Details

@samuelcolvin samuelcolvin deleted the timedelta-json-encoding branch Aug 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment