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

Consider exclude_none in computed_field serialization #768

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

hramezani
Copy link
Member

@hramezani hramezani commented Jul 13, 2023

Related issue: pydantic/pydantic#6549

Selected Reviewer: @adriangb

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #768 (f7037b3) into main (bf0bce0) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #768      +/-   ##
==========================================
- Coverage   93.66%   93.66%   -0.01%     
==========================================
  Files          99       99              
  Lines       14233    14247      +14     
  Branches       25       25              
==========================================
+ Hits        13331    13344      +13     
- Misses        896      897       +1     
  Partials        6        6              
Impacted Files Coverage Δ
src/serializers/computed_fields.rs 98.30% <100.00%> (+0.04%) ⬆️

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

@codspeed-hq
Copy link

codspeed-hq bot commented Jul 13, 2023

CodSpeed Performance Report

Merging #768 will not alter performance

Comparing computed_field_serialization_exclude_none (f7037b3) with main (bf0bce0)

Summary

✅ 126 untouched benchmarks

@hramezani
Copy link
Member Author

please review

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

LGTM

@samuelcolvin samuelcolvin merged commit 0c0e391 into main Jul 13, 2023
28 checks passed
@samuelcolvin samuelcolvin deleted the computed_field_serialization_exclude_none branch July 13, 2023 12:59
@enricotagliani
Copy link

I can confirm that the model_dump_json(exclude_none=True) case is not resolved yet as of latest pydantic version (2.4.2).

This behaviour can easily be replicated with the following code:

class Model(BaseModel):
    x: int
    y: int

    @computed_field
    def sum(self) -> int:
        return self.x + self.y

    @computed_field
    def none(self) -> None:
        return None


m = Model(x=1, y=2)
print(
    m.model_dump(exclude_none=True),
    m.model_dump_json(exclude_none=True),
    sep='\n'
)

# {'x': 1, 'y': 2, 'sum': 3}
# {"x":1,"y":2,"sum":3,"none":null}

@davidhewitt
Copy link
Contributor

@enricotagliani I can confirm this reproduces in 2.5.3 still, however I cannot reproduce on main, so I believe your case will be fixed on 2.6.0.

@majadesignz
Copy link

I can still reproduce it with 2.14.6. Is there any kind of workaround for that?

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

Successfully merging this pull request may close these issues.

6 participants