Skip to content

Fix ImportString special cases#10137

Merged
sydney-runkle merged 3 commits intomainfrom
test-import-string
Aug 15, 2024
Merged

Fix ImportString special cases#10137
sydney-runkle merged 3 commits intomainfrom
test-import-string

Conversation

@sydney-runkle
Copy link
Copy Markdown
Contributor

Fix #10122

I can understand the argument for a more general solution, but I wasn't able to find an easy workaround for io wrappers. Feedback welcome!

@sydney-runkle sydney-runkle requested a review from Viicos August 14, 2024 18:39
@github-actions github-actions Bot added the relnotes-fix Used for bugfixes. label Aug 14, 2024
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented Aug 14, 2024

Deploying pydantic-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3c5e4a0
Status: ✅  Deploy successful!
Preview URL: https://49125506.pydantic-docs.pages.dev
Branch Preview URL: https://test-import-string.pydantic-docs.pages.dev

View logs

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Aug 14, 2024

CodSpeed Performance Report

Merging #10137 will not alter performance

Comparing test-import-string (3c5e4a0) with main (e9f30dc)

Summary

✅ 17 untouched benchmarks

Comment thread tests/test_types.py Outdated
Comment thread pydantic/types.py
Comment on lines +1016 to +1017
# Handle special cases for sys.XXX streams
# if we see more of these, we should consider a more general solution
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think in general serializing the imported object to a string is a hard (even impossible without special casing as it is done here) problem, so I'm fine with the proposed solution.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Cool, thanks!

@github-actions
Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic
  types.py 1020, 1022, 1024
Project Total  

This report was generated by python-coverage-comment-action

@sydney-runkle sydney-runkle merged commit 9d55365 into main Aug 15, 2024
@sydney-runkle sydney-runkle deleted the test-import-string branch August 15, 2024 00:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

relnotes-fix Used for bugfixes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ImportString serialization fails for sys.stdout

2 participants