Skip to content

Conversation

sydney-runkle
Copy link
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
Copy link

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

Copy link

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 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
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
Contributor Author

Choose a reason for hiding this comment

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

Cool, thanks!

Copy link
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
61 checks passed
@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