Uh oh!
There was an error while loading. Please reload this page.
gh-89039: call subclass constructors in datetime.*.replace - #114780
Conversation
When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called. Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored.
ac7f8e2 to
b5ebbc5Compareeltoder
commented
Jan 31, 2024
@serhiy-storchaka This is a follow-up to #112921 to fix another issue. Would appreciate if you could take a look. |
serhiy-storchaka
left a comment
There was a problem hiding this comment.
LGTM. I have only few style nitpicks.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Thanks @eltoder for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Thanks @eltoder for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11. |
Sorry, @eltoder and @serhiy-storchaka, I could not cleanly backport this to |
Sorry, @eltoder and @serhiy-storchaka, I could not cleanly backport this to |
serhiy-storchaka
commented
Feb 12, 2024
Thank you for your contribution @eltoder. Do you mind to create a backport for 3.12? |
eltoder
commented
Feb 12, 2024
@serhiy-storchaka Thank you for merging. Can we just backport #112921? Then this will work automatically. Or should I write a version of this change that doesn't use argument clinic? |
serhiy-storchaka
commented
Feb 12, 2024
Usually we do not backport conversions to Argument Clinic and pure optimizations. Although if it is a straightforward backport that helps future backports, there may be exceptions. But seems that this is not the case. There are some conflicts in backporting of #112921, so they may be not so trivial. It may be safer to not backport it. |
eltoder
commented
Feb 12, 2024
@serhiy-storchaka The conflicts were very easy to resolve. I did it here: #115344 But I get a complaint from CI that the ABI changed. Is that correct or a false positive? |
serhiy-storchaka
commented
Feb 12, 2024
It does not allow to merge the PR without sanction of the Release Manager. |
eltoder
commented
Feb 12, 2024
@serhiy-storchaka What would you recommend? The backport itself is trivial, but it does change the size of pyruntimestate. Does this really affect the ABI? I see that there were some updates to it on the 3.12 branch by @ericsnowcurrently after 3.12 was released: https://github.com/python/cpython/commits/3.12/Doc/data/python3.12.abi |
ericsnowcurrently
commented
Feb 12, 2024
Correct. The runtime state struct is strictly internal, so we do sometimes change the related (internal) ABI in bug fix releases. |
eltoder
commented
Feb 13, 2024
@ericsnowcurrently Do you think it is acceptable to change pyruntimestate to backport #112921? I used argument clinic and it added some new string constants. |
ericsnowcurrently
commented
Feb 13, 2024
I don't see a problem with it, but release manager makes the final decision. |
…honGH-114780) When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called. Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored.
…ce (pythonGH-114780) When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called. Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored. (cherry picked from commit 46190d9) Co-authored-by: Eugene Toder <eltoder@users.noreply.github.com>
GH-131239 is a backport of this pull request to the 3.12 branch. |
…-114780) (GH-131239) When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called. Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored. (cherry picked from commit 46190d9) Co-authored-by: Eugene Toder <eltoder@users.noreply.github.com>
When replace() method is called on a subclass of datetime, date or time, properly call derived constructor. Previously, only the base class's constructor was called.
Also, make sure to pass non-zero fold values when creating subclasses in various methods. Previously, fold was silently ignored.