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
adds test case for unexpected discriminated union behavior #9236
Conversation
CodSpeed Performance ReportMerging #9236 will not alter performanceComparing Summary
|
9485af0
to
9da09f8
Compare
Thanks for adding the test case! Please remove all of the other changes made in this PR (reformatting), etc |
Ah can I just do make format for that?
…On Sat, Apr 13, 2024 at 17:22 Sydney Runkle ***@***.***> wrote:
Thanks for adding the test case! Please remove all of the other changes
made in this PR (reformatting), etc
—
Reply to this email directly, view it on GitHub
<#9236 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A5CBQT4PIORQ4P57S3QSVG3Y5HD4ZAVCNFSM6AAAAABGE5EMHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJTHAYTMNBWGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@kf-novi I believe so, or just revert the commit + add back the test if that doesn't work |
This reverts commit 9da09f8.
Sorry about the dual account thing here, opened with this one and keep forgetting which one I'm using. Anyway, had to revert since my auto-formatter added a bunch of commas that ruff format doesn't remove. So, should be good now. |
Maybe add an |
I will do that tomorrow!
…On Thu, Apr 18, 2024 at 17:50 Sydney Runkle ***@***.***> wrote:
@kf-novi <https://github.com/kf-novi>,
Maybe add an xfail with a link to your issue, then we can merge this?
—
Reply to this email directly, view it on GitHub
<#9236 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACUWLA2PSYGEWXJOM6PKMLY6BS5XAVCNFSM6AAAAABGE5EMHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRVGU2TIMRWGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, including this in the patch and hoping we can get a fix for this in 2.8
Change Summary
Adds failing test case for expected behavior in discriminated union
model_dump
Related issue number
#9235
Checklist