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

Prefer custom encoder over defaults if specified. #2567

Closed
wants to merge 1 commit into from
Closed

Prefer custom encoder over defaults if specified. #2567

wants to merge 1 commit into from

Conversation

viveksunder
Copy link
Contributor

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.

if type(obj) in custom_encoder:
return custom_encoder[type(obj)](obj)
else:
for encoder_type, encoder_instance in custom_encoder.items():
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mypy was getting confused because encoder is declared here in the for loop but also in the lines below. So I gave this var a different name here.

@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #2567 (fffd897) into master (5614b94) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2567   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          243       243           
  Lines         7419      7428    +9     
=========================================
+ Hits          7419      7428    +9     
Impacted Files Coverage Δ
fastapi/encoders.py 100.00% <100.00%> (ø)
tests/test_jsonable_encoder.py 100.00% <100.00%> (ø)

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 5614b94...fffd897. Read the comment docs.

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.

None yet

1 participant