Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

jsonable_encoder function not working as expected with different type of variables #2594

Closed
9 tasks done
xaviml opened this issue Jan 3, 2021 · 9 comments · Fixed by #2606
Closed
9 tasks done

jsonable_encoder function not working as expected with different type of variables #2594

xaviml opened this issue Jan 3, 2021 · 9 comments · Fixed by #2606
Labels
question Question or problem question-migrate

Comments

@xaviml
Copy link
Contributor

xaviml commented Jan 3, 2021

First check

  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the FastAPI documentation, with the integrated search.
  • I already searched in Google "How to X in FastAPI" and didn't find any information.
  • I already read and followed all the tutorial in the docs and didn't find an answer.
  • I already checked if it is not related to FastAPI but to Pydantic.
  • I already checked if it is not related to FastAPI but to Swagger UI.
  • I already checked if it is not related to FastAPI but to ReDoc.
  • After submitting this, I commit to one of:
    • Read open issues with questions until I find 2 issues where I can help someone and add a comment to help there.
    • I already hit the "watch" button in this repository to receive notifications and I commit to help at least 2 people that ask questions in the future.
    • Implement a Pull Request for a confirmed bug.

Example

Here's a self-contained, minimal, reproducible, example with my use case:

Example 1 (BaseModel)

import pytest
from attr import dataclass
from fastapi.encoders import jsonable_encoder
from pydantic.main import BaseModel

class MyBaseModel(BaseModel):
    foo: int
    bar: int


@pytest.mark.parametrize(
    "obj, include, exclude, expected",
    [
        (MyBaseModel(foo=1, bar=2), None, None, {"foo": 1, "bar": 2}),  # Passed
        (MyBaseModel(foo=1, bar=2), {}, {}, {}),  # Passed
        (MyBaseModel(foo=1, bar=2), {"foo"}, {}, {"foo": 1}),  # Passed
        (MyBaseModel(foo=1, bar=2), {}, {"foo"}, {}),  # Passed
        (MyBaseModel(foo=1, bar=2), {"foo"}, None, {"foo": 1}),  # Passed
        (MyBaseModel(foo=1, bar=2), None, {"foo"}, {"bar": 2}),  # Passed
    ],
)
def test_jsonable_encoder_include_exclude_base_model(obj, include, exclude, expected):
    assert jsonable_encoder(obj, include=include, exclude=exclude) == expected

Example 2 (Python dictionary)

import pytest
from fastapi.encoders import jsonable_encoder


@pytest.mark.parametrize(
    "obj, include, exclude, expected",
    [
        ({"foo": 1, "bar": 2}, None, None, {"foo": 1, "bar": 2}),  # Passed
        ({"foo": 1, "bar": 2}, {}, {}, {}),  # Failed
        ({"foo": 1, "bar": 2}, {"foo"}, {}, {"foo": 1}),  # Failed
        ({"foo": 1, "bar": 2}, {}, {"foo"}, {}),  # Failed
        ({"foo": 1, "bar": 2}, {"foo"}, None, {"foo": 1}),  # Failed
        ({"foo": 1, "bar": 2}, None, {"foo"}, {"bar": 2}),  # Passed
    ],
)
def test_jsonable_encoder_include_exclude_dict(obj, include, exclude, expected):
    assert jsonable_encoder(obj, include=include, exclude=exclude) == expected

Example 3 (Python object with dataclass)

@dataclass
class MyModel:
    foo: int
    bar: int


@pytest.mark.parametrize(
    "obj, include, exclude, expected",
    [
        (MyModel(foo=1, bar=2), None, None, {"foo": 1, "bar": 2}),  # Passed
        (MyModel(foo=1, bar=2), {}, {}, {}),  # Failed
        (MyModel(foo=1, bar=2), {"foo"}, {}, {"foo": 1}),  # Failed
        (MyModel(foo=1, bar=2), {}, {"foo"}, {}),  # Failed
        (MyModel(foo=1, bar=2), {"foo"}, None, {"foo": 1}),  # Failed
        (MyModel(foo=1, bar=2), None, {"foo"}, {"bar": 2}),  # Failed
    ],
)
def test_jsonable_encoder_include_exclude_obj(obj, include, exclude, expected):
    assert jsonable_encoder(obj, include=include, exclude=exclude) == expected

Description

I was trying to understand how the include and exclude parameters worked. I looked in the documentation (here) and the the test suite (here, latest commit of this file in master branch) and I did not find anything related to include and exclude in both places.

I did see that these 2 parameters came from Pydantic (from the dict function) and it is being used in the _calculate_keys method.

The first thing I did was to check the jsonable_encoder using a BaseModel object, which uses the dict function from Pydantic (Example 1), so I created a set of test with a BaseModel instance. All those tests are passing. Then I created the same tests with a Python dictionary (Example 2) and a Python object (Example 3).

I expect that the same tests with a different type of variable would work the same. In the case of Example 3, it is failing the most because it is missing the include and exclude parameters at the end of the function (here).

Environment

  • OS: Linux
  • Python: 3.8.5
  • FastAPI Version: 0.63.0

To know the FastAPI version use:

python -c "import fastapi; print(fastapi.__version__)"
  • Python version:

To know the Python version use:

python --version

Additional context

To reproduce the Example snippets, you will also need pytest installed (6.2.1 in my case)
I am happy to help by creating a PR for this bug once it is confirmed.

All examples together
import pytest
from attr import dataclass
from fastapi.encoders import jsonable_encoder
from pydantic.main import BaseModel


class MyBaseModel(BaseModel):
  foo: int
  bar: int


@dataclass
class MyModel:
  foo: int
  bar: int


@pytest.mark.parametrize(
  "obj, include, exclude, expected",
  [
      (MyBaseModel(foo=1, bar=2), None, None, {"foo": 1, "bar": 2}),  # Passed
      (MyBaseModel(foo=1, bar=2), {}, {}, {}),  # Passed
      (MyBaseModel(foo=1, bar=2), {"foo"}, {}, {"foo": 1}),  # Passed
      (MyBaseModel(foo=1, bar=2), {}, {"foo"}, {}),  # Passed
      (MyBaseModel(foo=1, bar=2), {"foo"}, None, {"foo": 1}),  # Passed
      (MyBaseModel(foo=1, bar=2), None, {"foo"}, {"bar": 2}),  # Passed
      ({"foo": 1, "bar": 2}, None, None, {"foo": 1, "bar": 2}),  # Passed
      ({"foo": 1, "bar": 2}, {}, {}, {}),  # Failed
      ({"foo": 1, "bar": 2}, {"foo"}, {}, {"foo": 1}),  # Failed
      ({"foo": 1, "bar": 2}, {}, {"foo"}, {}),  # Failed
      ({"foo": 1, "bar": 2}, {"foo"}, None, {"foo": 1}),  # Failed
      ({"foo": 1, "bar": 2}, None, {"foo"}, {"bar": 2}),  # Passed
      (MyModel(foo=1, bar=2), None, None, {"foo": 1, "bar": 2}),  # Passed
      (MyModel(foo=1, bar=2), {}, {}, {}),  # Failed
      (MyModel(foo=1, bar=2), {"foo"}, {}, {"foo": 1}),  # Failed
      (MyModel(foo=1, bar=2), {}, {"foo"}, {}),  # Failed
      (MyModel(foo=1, bar=2), {"foo"}, None, {"foo": 1}),  # Failed
      (MyModel(foo=1, bar=2), None, {"foo"}, {"bar": 2}),  # Failed
  ],
)
def test_jsonable_encoder_include_exclude(obj, include, exclude, expected):
  assert jsonable_encoder(obj, include=include, exclude=exclude) == expected
@xaviml xaviml added the question Question or problem label Jan 3, 2021
@falkben
Copy link
Sponsor Contributor

falkben commented Jan 3, 2021

The pydantic docs for include/exclude are here, for anyone curious regarding this issue.

@xaviml How would you expect this to work for objects that are not pydantic models? Seems like we'd have to make a lot of assumptions about the structure of the objects.

Perhaps docs could be added saying include/exclude are only valid for pydantic models?

Trying to get some more information: If you really want to use include/exclude, is there anything preventing you from using a pydantic model.

@xaviml
Copy link
Contributor Author

xaviml commented Jan 3, 2021

Hi @falkben,

Thank you for your reply.

How would you expect this to work for objects that are not pydantic models?

I would expect to be consistent, so to work the same as Pydantic models. For example, if we do:

jsonable_encoder({"foo": 1, "bar": 2}, include={"foo"})

I would expect to return {"foo": 1}. However, this returns: {"foo": 1, "bar": 2}. The same way that if we do:

jsonable_encoder({"foo": 1, "bar": 2}, exclude={"foo"})

It returns {"bar": 2} as expected.

Seems like we'd have to make a lot of assumptions about the structure of the objects.

I don't think is about the assumption of the structure of the object. They are all converted to dictionaries with dict (here) or vars(here). What needs to change is the way of including and excluding (here).

Perhaps docs could be added saying include/exclude are only valid for pydantic models?

You can see in this if branch that include and exclude is also used when the object is a dictionary.

Trying to get some more information: If you really want to use include/exclude, is there anything preventing you from using a pydantic model.

I am using the jsonable_encoder with a SQLAlchemy object, the same way it is done in here. This is what is preventing me to use a Pydantic model, because I use a SQLAlchemy model. I will find a workaround to this, but this is another matter not related to this issue.

When I created this issue I tried to understand what is the expected use of include and exclude parameters from jsonable_encoder. This is why I went to the tests and see what is the expected behaviour of this function with the use of include and exclude. What I proposed is how I think it should be (and how it works with Pydantic), but I am happy to hear and discuss the right implementation for this function with these parameters.

Regards,
Xavi M.

@falkben
Copy link
Sponsor Contributor

falkben commented Jan 3, 2021

Possibly related? #2016

@xaviml
Copy link
Contributor Author

xaviml commented Jan 3, 2021

Hi @falkben,

I did see that issue before creating this one. The #2016 PR adds support to dictionaries for the include and exclude, but it does.not change the way include and exclude are being used. Although both of these issues (#2016 and this one) are about jsonable_encoder, they are not related.

Regards,
Xavi M.

@xaviml xaviml changed the title jsonable_encoder function not working as expected with different type of variables [BUG] jsonable_encoder function not working as expected with different type of variables Jan 5, 2021
xaviml added a commit to xaviml/fastapi that referenced this issue Jan 5, 2021
This fixes tiangolo#2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.
@xaviml
Copy link
Contributor Author

xaviml commented Jan 5, 2021

I created a PR (#2606) fixing the problem mentioned here. You can find in the PR what the problem was and which tests were affected.

@V3lop5
Copy link

V3lop5 commented Dec 15, 2021

I ran in the same bug tonight.
Any chance that this issue gets classified as bug and the corresponding pr gets reviewed? @tiangolo

@stapetro
Copy link

Hello,

I've added 1 more test case in another issue for proving the change introduced in 0.67 version broke the contract of jsonable_encoder.

@Elementor-studios
Copy link

Elementor-studios commented Jan 19, 2022 via email

@oiricaud
Copy link

Having this issue...

xaviml added a commit to xaviml/fastapi that referenced this issue Apr 18, 2022
This fixes tiangolo#2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.
xaviml added a commit to xaviml/fastapi that referenced this issue May 3, 2022
This fixes tiangolo#2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.
xaviml added a commit to xaviml/fastapi that referenced this issue May 8, 2022
This fixes tiangolo#2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.
xaviml added a commit to xaviml/fastapi that referenced this issue May 22, 2022
This fixes tiangolo#2594. I added more few cases with include and exclude parameters, which were missing. Now the parameters are consistent with the way that it works with Pydantic.
@tiangolo tiangolo changed the title [BUG] jsonable_encoder function not working as expected with different type of variables jsonable_encoder function not working as expected with different type of variables Feb 24, 2023
@tiangolo tiangolo reopened this Feb 27, 2023
Repository owner locked and limited conversation to collaborators Feb 27, 2023
@tiangolo tiangolo converted this issue into discussion #6942 Feb 27, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Question or problem question-migrate
Projects
None yet
7 participants