🐛 Prefer custom encoder over defaults if specified in `jsonable_encoder` #2061

viveksunder · 2020-09-16T04:49:08Z

In encoders.py/jsonable_encoder, default encoders for each type are specified first & are used in preference to custom encoders.

This affects custom encoders specified for certain types like enums such that the default encoder is always executed and any custom encoders specified are ignored. See - #1986 - for a description of this issue and example code as well that illustrates the problem.

This PR adds a change to prefer custom encoders if specified in preference to default encoders.

Disclaimer - I am completely new to this code base :-)

codecov · 2020-09-16T04:50:38Z

Codecov Report

Merging #2061 (fffd897) into master (3de0fb8) will not change coverage.
The diff coverage is 100.00%.

❗ Current head fffd897 differs from pull request most recent head a7efa6a. Consider uploading reports for the commit a7efa6a to get more accurate results

@@             Coverage Diff             @@
##            master     #2061     +/-   ##
===========================================
  Coverage   100.00%   100.00%             
===========================================
  Files          509       243    -266     
  Lines        12807      7428   -5379     
===========================================
- Hits         12807      7428   -5379

Impacted Files	Coverage Δ
fastapi/encoders.py	`100.00% <100.00%> (ø)`
tests/test_jsonable_encoder.py	`100.00% <100.00%> (ø)`
fastapi/params.py	`100.00% <0.00%> (ø)`
fastapi/routing.py	`100.00% <0.00%> (ø)`
fastapi/responses.py	`100.00% <0.00%> (ø)`
fastapi/concurrency.py	`100.00% <0.00%> (ø)`
fastapi/applications.py	`100.00% <0.00%> (ø)`
fastapi/openapi/docs.py	`100.00% <0.00%> (ø)`
fastapi/openapi/utils.py	`100.00% <0.00%> (ø)`
... and 291 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3de0fb8...a7efa6a. Read the comment docs.

github-actions · 2020-09-16T07:13:30Z

📝 Docs preview for commit e85b8e86880ef1da23a4c2e6a39535499c8eb3ff at: https://5f61bb0c6d3cd4463e8e5c14--fastapi.netlify.app

github-actions · 2020-09-16T23:11:42Z

📝 Docs preview for commit 379762189f4ac877ae9ec32214fc0008a6fe40ba at: https://5f629ba0924ff0920a89f626--fastapi.netlify.app

viveksunder · 2020-09-22T23:02:52Z

Hi @ycd - thanks for approving. Just wondering when PRs are typically merged in & what the process is. Is there anything else needed from me in order to get this merged in?

Kludex · 2020-09-23T06:12:44Z

Just wait for @tiangolo 's input.

ycd · 2020-09-23T06:52:52Z

Exactly, @tiangolo usually releases a new version in 14-21 days, but he just recently moved to Germany so he is probably too busy too look here, but i believe next release would not take longer than 1-2 weeks.

viveksunder · 2020-09-23T17:30:20Z

Gotcha, not a problem at all! Thanks @ycd & @Kludex :)

mikegrima · 2020-09-25T16:54:09Z

Hello! This does appear to fix my issue at #2091. However, if I may make one suggestion to this PR, please also fix the mutable dictionary that is set as a keyword argument with this code:

    custom_encoder: dict = None,     # <------- Make this change
    sqlalchemy_safe: bool = True,
) -> Any:
    custom_encoder = custom_encoder or {}       # <-------- And this change
    if custom_encoder:
        if type(obj) in custom_encoder:
            return custom_encoder[type(obj)](obj)
        else:
            for encoder_type, encoder in custom_encoder.items():
                if isinstance(obj, encoder_type):
                    return encoder(obj)

More details on this is here: https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments

viveksunder · 2020-09-25T17:28:22Z

Hello! This does appear to fix my issue at #2091. However, if I may make one suggestion to this PR, please also fix the mutable dictionary that is set as a keyword argument with this code:

    custom_encoder: dict = None,     # <------- Make this change
    sqlalchemy_safe: bool = True,
) -> Any:
    custom_encoder = custom_encoder or {}       # <-------- And this change
    if custom_encoder:
        if type(obj) in custom_encoder:
            return custom_encoder[type(obj)](obj)
        else:
            for encoder_type, encoder in custom_encoder.items():
                if isinstance(obj, encoder_type):
                    return encoder(obj)

More details on this is here: https://docs.python-guide.org/writing/gotchas/#mutable-default-arguments

I agree! I can make this change if there are no objections.

github-actions · 2020-09-29T17:13:37Z

📝 Docs preview for commit a92f760b66ecf936ad3bee568c7115220a3e3da8 at: https://5f736b1a6789f9014b8f9245--fastapi.netlify.app

viveksunder · 2020-10-14T17:43:06Z

Hate to be a bother @ycd :) but just bumping this because I made the change @mikegrima suggested.

Also, do we think this is going to be merged in soon? Trying to decide if I should use my personal fork to use this change (which I'd rather not on the off-chance that this change doesn't actually get merged in)

mikegrima · 2020-10-24T01:48:58Z

Would like to make a friendly bump to get this released soon.

viveksunder · 2020-10-28T19:37:27Z

Hi @tiangolo - We (and others) would really like to use this ASAP. Really appreciate it if you could review this & lmk if good.

viveksunder · 2020-12-05T00:28:18Z

Hello. Is there something preventing this from being merged? It's been open for a while now and we really need this change!

mikegrima · 2020-12-23T15:36:16Z

@viveksunder Can you rebase this PR?

@ycd one this is rebased, can you merge it in?

github-actions · 2020-12-24T03:57:26Z

📝 Docs preview for commit 3a89879740e47c8760f9ed863e59b5ebef9fc253 at: https://5fe41162b6d01b7e356fabee--fastapi.netlify.app

viveksunder · 2020-12-24T04:59:46Z

~~My git commit history got borked while rebasing 🤕 so I started a new one here - #2567~~

Never mind, github is pretty smart & turns out I didn't need to open a new one.

github-actions · 2020-12-24T05:01:17Z

📝 Docs preview for commit fffd897 at: https://5fe42059b6d01baf796fa7df--fastapi.netlify.app

github-actions · 2020-12-24T05:04:33Z

📝 Docs preview for commit fffd897 at: https://5fe42118605fe19c1aa29afe--fastapi.netlify.app

ycd · 2020-12-25T13:28:23Z

@mikegrima I'd love to do it but the only @tiangolo can merge PR's.

mojimi · 2021-02-08T19:37:27Z

Really looking forward to this, any news from @tiangolo ?

mikegrima · 2021-02-16T03:52:18Z

@tiangolo Friendly bump to merge.

dhofstetter · 2021-03-16T20:24:43Z

@tiangolo Do you see any problems with the approach implemented here? Sometimes it's really useful to override the default encoder. E.g. encoding Decimals to json using a predefined quantization. Would be really great if you would accept this PR

kovalevvlad · 2021-03-18T19:29:02Z

+1 would really like to have a custom Enum serializer. @tiangolo

mikegrima · 2021-03-18T19:30:17Z

@tiangolo please click merge. The PR fixes a bug.

EDIT: Sorry if this came off too strong (that was not my intention).

philipbjorge · 2021-05-06T18:30:04Z

Is this PR still necessary?

For my usecase at least, it seems like the following resolved.
#1769

Probably there's a different one I'm not understanding :)

kovalevvlad · 2021-05-07T20:03:26Z

@philipbjorge indeed it is still needed, the check for isinstance(obj, Enum) in fastapi/encoders.py is performed before checking whether custom encoders exist, therefore it is currently impossible to create a functional custom encoder for an Enum type.

mojimi · 2021-05-17T14:17:11Z

For me this is very necessary, as I work with DynamoDB and it requires floats to be transformed to Decimal in the boto3 library.

My current solution makes another iteration on top of the result of jsonable_encoder to fix floats, quite unoptimal

mikegrima · 2021-05-17T21:23:14Z

Can someone please reach out to @tiangolo to get this merged in? This fixes a legitimate bug and adds a desirable feature for a number of users.

JP-Globality · 2021-05-23T15:47:04Z

@tiangolo is there anything else we can do to get this PR over the line and merged in?

I too have the same issue as @mikegrima and can verify this PR fixes my issue by allowing the user to specify custom json_encoders for Enums. Simply doing enum.value doesn't produce JSONable values for nested Enums.

darkpssngr · 2021-08-05T08:25:32Z

waiting for this to be merged as well. @tiangolo could you please look at it?

justinttl · 2021-09-08T14:43:20Z

It will be fantastic to iron out this very surprising behavior and incompatibility between FastAPI and pydantic! Trying to implement pydantic/pydantic#1004 for my API at the moment and it relies on custom encoders working expectedly. @tiangolo

pdenapo · 2021-10-05T16:08:00Z

+1 It will be very helpful to fix this. For instance, I'm trying to use fastapi with Mongodb, and I'am having trouble with the Decimal128 type for pymongo.bson which is not natively supported by pydantic, so I would need to provide a custom serialization. The problem seems to be with collection types like sets, lists, etc. @tiangolo

Initial project setup * Add module with settings * Add EAV models * Add some CRUD actions and pydantic models for them * Add `commit` flag to create_attribute(): Need this because we can create `Attribute` during `Schema` creation, but something can go wrong, so we don't want it to be saved immediately * Allow non-unique values for entities: Now we can create entities with unique attributes having non-unique values if existing entity(ies) having this value is marked as deleted * Forbid binding FK to deleted schemas * Forbid updates of deteled schemas * Forbid creating entities for deleted schemas * Add link to entity for Value models * Add unique constraint on AttributeDefinition * Add custom exceptions * Change DT caster to `fromtimestamp` * Add AttrTypeMap as serializing workaround: Currently, if we try to call `.json()` on any Pydantic model that has `AttrType` field in it, Pydantic won't be able to convert it. Pydantic allows custom serializers for type, so we can overcome this issue for Pydantic. However, it seems that FastAPI ignores custom serializers and breaks when it comes across our custom types. That's why I wrote this workaround. There will be no need in it when this (or similar) PR gets merged to FastAPI repo: fastapi/fastapi#2061 * Added some tests for crud * Check required fields on entity creation * Catch IntegrityError on update of schema name/slug

nicknotfun · 2021-10-15T21:43:40Z

Also waiting on this fix, wondering what is holding it up? @ycd @JP-Globality @tiangolo ?

aliereno · 2021-11-09T10:56:30Z

I needed to implement custom encoder for decimal and enum types, but I couldn't. I am looking forward to this PR merged.

…rges see: fastapi/fastapi#2606 see: fastapi/fastapi#2061

yribeiro · 2022-01-06T15:52:56Z

@tiangolo please merge this. My team rely on FastAPI and Pydantic quite heavily. Having to write custom types and using type ignores is not the best. Would be great if this can get merged into the next release!

github-actions · 2022-01-23T16:32:18Z

📝 Docs preview for commit a7efa6a at: https://61ed82f4bc4aa1c45bb3b669--fastapi.netlify.app

tiangolo · 2022-01-23T16:33:15Z

Awesome, thank you @viveksunder! ☕

And thanks everyone for the interest! 🍰

This is available in FastAPI 0.73.0, released in a couple of hours. 🎉

…er` (fastapi#2061) Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>

ycd approved these changes Sep 17, 2020

View reviewed changes

ycd mentioned this pull request Sep 25, 2020

FastAPI ignores json_encoders in the Model #2091

Closed

includeamin mentioned this pull request Dec 2, 2020

OpenAPI generation failed with internal enums and with pydantic >=1.6.* #2457

Closed

2 tasks

viveksunder closed this Dec 24, 2020

Prefer custom encoder over defaults if specified.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

fffd897

github-actions bot temporarily deployed to commit December 24, 2020 05:00 Inactive

viveksunder reopened this Dec 24, 2020

JP-Globality approved these changes May 27, 2021

View reviewed changes

V3lop5 added a commit to NOWUM/EnSysMod that referenced this pull request Jan 4, 2022

Added modified version of fastapis jsonable_encoder due to pending me…

05ba946

…rges see: fastapi/fastapi#2606 see: fastapi/fastapi#2061

tiangolo added 2 commits January 23, 2022 17:21

🎨 Add type annotation to custom encoder

a195ed4

🔀 Merge master

a7efa6a

github-actions bot temporarily deployed to commit January 23, 2022 16:31 Inactive

tiangolo mentioned this pull request Jan 23, 2022

🐛 Prefer custom encoder over defaults if specified in jsonable_encoder #4467

Merged

tiangolo changed the title ~~Prefer custom encoder over defaults if specified.~~ 🐛 Prefer custom encoder over defaults if specified in jsonable_encoder Jan 23, 2022

tiangolo merged commit 94ca8c1 into fastapi:master Jan 23, 2022

JeanArhancet pushed a commit to JeanArhancet/fastapi that referenced this pull request Aug 20, 2022

🐛 Prefer custom encoder over defaults if specified in `jsonable_encod…

0c2a3f3

…er` (fastapi#2061) Co-authored-by: Sebastián Ramírez <tiangolo@gmail.com>

🐛 Prefer custom encoder over defaults if specified in jsonable_encoder #2061

🐛 Prefer custom encoder over defaults if specified in jsonable_encoder #2061

Conversation

viveksunder commented Sep 16, 2020

codecov bot commented Sep 16, 2020 • edited Loading

Codecov Report

github-actions bot commented Sep 16, 2020

github-actions bot commented Sep 16, 2020

viveksunder commented Sep 22, 2020

Kludex commented Sep 23, 2020

ycd commented Sep 23, 2020

viveksunder commented Sep 23, 2020

mikegrima commented Sep 25, 2020 • edited Loading

viveksunder commented Sep 25, 2020

github-actions bot commented Sep 29, 2020

viveksunder commented Oct 14, 2020

mikegrima commented Oct 24, 2020

viveksunder commented Oct 28, 2020

viveksunder commented Dec 5, 2020

mikegrima commented Dec 23, 2020

github-actions bot commented Dec 24, 2020

viveksunder commented Dec 24, 2020 • edited Loading

github-actions bot commented Dec 24, 2020

github-actions bot commented Dec 24, 2020

ycd commented Dec 25, 2020

mojimi commented Feb 8, 2021

mikegrima commented Feb 16, 2021

dhofstetter commented Mar 16, 2021

kovalevvlad commented Mar 18, 2021 • edited Loading

mikegrima commented Mar 18, 2021 • edited Loading

philipbjorge commented May 6, 2021 • edited Loading

kovalevvlad commented May 7, 2021

mojimi commented May 17, 2021

mikegrima commented May 17, 2021 • edited Loading

JP-Globality commented May 23, 2021

darkpssngr commented Aug 5, 2021

justinttl commented Sep 8, 2021

pdenapo commented Oct 5, 2021 • edited Loading

nicknotfun commented Oct 15, 2021

aliereno commented Nov 9, 2021

yribeiro commented Jan 6, 2022

github-actions bot commented Jan 23, 2022

tiangolo commented Jan 23, 2022

🐛 Prefer custom encoder over defaults if specified in `jsonable_encoder` #2061

🐛 Prefer custom encoder over defaults if specified in `jsonable_encoder` #2061

codecov bot commented Sep 16, 2020 •

edited

Loading

mikegrima commented Sep 25, 2020 •

edited

Loading

viveksunder commented Dec 24, 2020 •

edited

Loading

kovalevvlad commented Mar 18, 2021 •

edited

Loading

mikegrima commented Mar 18, 2021 •

edited

Loading

philipbjorge commented May 6, 2021 •

edited

Loading

mikegrima commented May 17, 2021 •

edited

Loading

pdenapo commented Oct 5, 2021 •

edited

Loading