homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: datetime.py implementation of .replace inconsistent with C implementation
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: p-ganssle, r.david.murray, steven.daprano, vstinner
Priority: normal Keywords: patch

Created on 2017年08月16日 17:01 by p-ganssle, last changed 2022年04月11日 14:58 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4176 merged p-ganssle, 2017年10月30日 17:08
PR 4356 merged python-dev, 2017年11月09日 21:36
Messages (8)
msg300377 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2017年08月16日 17:01
In the .py implementation of datetime.replace (and date.replace and time.replace), the new datetime is created using the datetime type:
https://github.com/python/cpython/blob/master/Lib/datetime.py#L1578
But in the C source, it is created from type(self):
https://github.com/python/cpython/blob/master/Modules/_datetimemodule.c#L5046
I think the second should be the preferred behavior, so the datetime.py source should be updated to reflect that it's calling self.__class__(...) rather than datetime(...). I can prepare a PR if this would be desirable.
(That said, I'm not 100% clear under what circumstances the code in datetime.py is actually *used*, so I'm not sure how to write tests for it - is datetime.py essentially documentation, or is there a way to explicitly fall back to it?)
Per this issue on the pypy3 tracker:
https://bitbucket.org/pypy/pypy/issues/2635/datetimereplace-always-returns 
msg300378 - (view) Author: Steven D'Aprano (steven.daprano) * (Python committer) Date: 2017年08月16日 17:13
I agree that returning type(self) or self.__class__ (not sure which is better) is the right thing to do.
It might be possible to argue that the Python version is buggy, if the C version is treated as the reference implementation and the Python version has to duplicate that. Or if the documentation specifies the C version's behaviour.
But if the precise behaviour is unspecified, or if the Python version is the reference implementation, its best to treat this as an enhancement rather than a bug fix and only apply it to 3.7.
I'm not sure which is the right approach here.
msg300379 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2017年08月16日 17:15
See also issue 20371.
msg300382 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2017年08月16日 17:39
@r.david.murray In the other thread, you mention that the full test suite is run against the C and Python implementations, so that answers the question of how to write the tests.
I think treating it as an enhancement in Python 3.7 makes a reasonable amount of sense - it's clearly under-specified at the moment and people are probably relying on the CPython behavior (dateutil definitely is in the latest stable release, but not on master). Saying "it's implementation-specific before Python 3.7 but in Python 3.7+, the spec says it should use self(type)" is fine by me. It's not particularly hard to work around if you're subclassing datetime anyway.
Among the major libraries that provide their own datetime objects:
- Arrow seems to use composition (https://github.com/crsmithdev/arrow/blob/master/arrow/arrow.py)
- pendulum subclasses, but implements their own "replace": https://github.com/sdispater/pendulum/blob/master/pendulum/pendulum.py#L25
- delorean uses composition: https://github.com/myusuf3/delorean/blob/master/delorean/dates.py#L174
- maya uses composition: https://github.com/kennethreitz/maya/blob/master/maya/core.py#L72
I'd say for the most part it's not a major issue to change it even as a bugfix, particularly if the line we're going with is "it was always implementation-specific", but there's also no rush.
msg305240 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2017年10月30日 17:11
Some time ago this was fixed in pypy3:
https://bitbucket.org/pypy/pypy/issues/2635/datetimereplace-always-returns
I made a PR fixing this for `datetime`, `date` and `time`.
msg305984 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年11月09日 21:34
New changeset 191e993365ac3206f46132dcf46236471ec54bfa by Victor Stinner (Paul Ganssle) in branch 'master':
bpo-31222: Make (datetime|date|time).replace return subclass type in Pure Python (#4176)
https://github.com/python/cpython/commit/191e993365ac3206f46132dcf46236471ec54bfa
msg306000 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年11月10日 00:52
New changeset b9a40aca2935d2569191844c88f8b61269e383cb by Victor Stinner (Miss Islington (bot)) in branch '3.6':
bpo-31222: Make (datetime|date|time).replace return subclass type in Pure Python (GH-4176) (#4356)
https://github.com/python/cpython/commit/b9a40aca2935d2569191844c88f8b61269e383cb
msg306017 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017年11月10日 10:21
Thanks Paul Ganssle for the bugfix! I merged your PR and backported it to Python 3.6. (Python 3.5 doesn't accept bugfixes anymore.)
History
Date User Action Args
2022年04月11日 14:58:50adminsetgithub: 75405
2018年12月04日 17:21:38belopolskylinkissue20371 superseder
2017年11月10日 10:21:22vstinnersetstatus: open -> closed
versions: - Python 3.5
messages: + msg306017

resolution: fixed
stage: patch review -> resolved
2017年11月10日 00:52:07vstinnersetmessages: + msg306000
2017年11月09日 21:36:08python-devsetpull_requests: + pull_request4312
2017年11月09日 21:34:35vstinnersetnosy: + vstinner
messages: + msg305984
2017年10月30日 17:11:14p-gansslesetmessages: + msg305240
2017年10月30日 17:08:07p-gansslesetkeywords: + patch
stage: patch review
pull_requests: + pull_request4145
2017年08月16日 17:39:48p-gansslesetmessages: + msg300382
2017年08月16日 17:15:22r.david.murraysetnosy: + r.david.murray
messages: + msg300379
2017年08月16日 17:13:13steven.dapranosetnosy: + steven.daprano
messages: + msg300378
2017年08月16日 17:01:45p-gansslecreate

AltStyle によって変換されたページ (->オリジナル) /