gh-142214: Fix two regressions in dataclasses#142223
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
DavidCEllis commented Dec 3, 2025
I am kicking myself on the It looks like the sqlalchemy field issue is from replacing and subsequently attempting to restore annotations in order to add fields to a dataclass as there's no mechanism for doing so without annotations. The check to fix this is fine, I'm actually surprised I hadn't written this as a comprehension. Again, sorry for missing these possibilities. |
Co-authored-by: sobolevn <[email protected]>
There was a problem hiding this comment.
I just locally applied the changes in Lib/dataclasses.py and it fixed all the failing tests on github.com/pipefunc/pipefunc.
ericvsmith left a comment
ericvsmith left a comment There was a problem hiding this comment.
A small nit, but otherwise looks good to me.
| # gh-142214: The annotation may be missing in unusual dynamic cases. | ||
| # If so, just skip it. | ||
| if k in cls_annotations: | ||
| new_annotations[k] = cls_annotations[k] |
There was a problem hiding this comment.
Don't in and [] both do a lookup? Maybe it would be better as:
ifann:=cls_annotations.get(k): new_annotations[k] =ann
There was a problem hiding this comment.
I believe that None is a valid annotation here so this would miss None values?
There was a problem hiding this comment.
Correct. To save a lookup we could do a try-except KeyError instead. I don't have a preference but I can switch to that if you like.
There was a problem hiding this comment.
I think try/except makes sense as in most cases the key should be there. The key not being there should only be in cases where something has modified annotations and removed things after the class has been processed.
When you're done making the requested changes, leave the comment: |
JelleZijlstra commented Dec 4, 2025
I have made the requested changes; please review again |
Thanks for making the requested changes! @ericvsmith: please review the changes made to this pull request. |
JelleZijlstra merged commit 53ec7c8 into python:mainUh oh!
There was an error while loading. Please reload this page.
Thanks @JelleZijlstra for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
(cherry picked from commit 53ec7c8) Co-authored-by: Jelle Zijlstra <[email protected]>
GH-142277 is a backport of this pull request to the 3.14 branch. |
6 participants
__init__no longer work in 3.14.1 due to #137711 #142214