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

Update handling for Pydantic objects in mf_pformat #1666

Merged
merged 2 commits into from
Feb 10, 2025
Merged

Conversation

plypaul
Copy link
Contributor

@plypaul plypaul commented Feb 7, 2025

This PR updates the handling for Pydantic objects in mf_pformat to check if the dict() methods exists instead of the isinstance(BaseModel) check. The BaseModel is actually from the dsi_shim and in some cases with different Pydantic versions, it's possible that the check does not pass. Since it's not foolproof, the formatter falls back to pprint if there is an error.

@cla-bot cla-bot bot added the cla:yes label Feb 7, 2025
@plypaul plypaul marked this pull request as ready for review February 7, 2025 02:57
@plypaul plypaul requested a review from a team as a code owner February 7, 2025 02:57
remaining_line_width=remaining_line_width,
)
# For Pydantic-like objects with a `dict` method that returns field keys / values.
dict_method = getattr(obj, "dict", None)

Choose a reason for hiding this comment

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

@plypaul isn't there a chance that it breaks with pydantic v2 where dict was deprecated in favor of model_dump ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It actually still works, though now I see the deprecation warning. Updated the case handling though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also to provide context on these formatting methods - in case there's a problem formatting, it falls back to the standard pretty printer or str(...).

Base automatically changed from p--misc--01 to main February 7, 2025 18:01
@plypaul plypaul merged commit d91ef7e into main Feb 10, 2025
11 checks passed
@plypaul plypaul deleted the p--misc--02 branch February 10, 2025 20:43
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.

4 participants