[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[STINNER Victor]

STINNER Victor  added the comment:

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.)

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
versions:  -Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[STINNER Victor]

STINNER Victor  added the comment:


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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Roundup Robot]

Change by Roundup Robot :


--
pull_requests: +4312

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[STINNER Victor]

STINNER Victor  added the comment:


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


--
nosy: +haypo

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Paul Ganssle]

Paul Ganssle  added the comment:

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`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Paul Ganssle]

Change by Paul Ganssle :


--
keywords: +patch
pull_requests: +4145
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Paul G]

Paul G added the comment:

@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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[R. David Murray]

R. David Murray added the comment:

See also issue 20371.

--
nosy: +r.david.murray

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Steven D'Aprano]

Steven D'Aprano added the comment:

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.

--
nosy: +steven.daprano

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue31222] datetime.py implementation of .replace inconsistent with C implementation

[Paul G]

New submission from Paul G:

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

--
components: Interpreter Core
messages: 300377
nosy: p-ganssle
priority: normal
severity: normal
status: open
title: datetime.py implementation of .replace inconsistent with C implementation
versions: Python 3.5, Python 3.6, Python 3.7

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com