classification
Title: Change time.mktime() return type from float to int?
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.8
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: belopolsky, p-ganssle, vstinner
Priority: normal Keywords: patch

Created on 2019-04-08 12:54 by vstinner, last changed 2019-04-16 16:18 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12727 closed vstinner, 2019-04-08 13:04
Messages (4)
msg339632 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-08 12:54
time.mktime() returns a floating point number:

>>> type(time.mktime(time.localtime()))
<class 'float'>

The documentation says:

"It returns a floating point number, for compatibility with :func:`.time`."

time.time() returns a float because it has sub-second resolution, but mktime() returns an integer number of seconds.

Would it make sense to change mktime() return type from float to int?

I would like to change mktime() return type to make the function more consistent: inputs are integers, it sounds wrong to me to return float. The result should be integer as well.

How much code would it break? I guess that the main impact are unit tests relying on repr(time.mktime(t)) exact value. But it's easy to fix the tests: use int(time.mktime(t)) or "%.0f" % time.mktime(t) to never get ".0", or use float(time.mktime(t))) to explicitly cast for a float (that which be a bad but quick fix).

Note: I wrote and implemented the PEP 564 to avoid any precision loss. mktime() will not start loosing precision before year 285,422,891 (which is quite far in the future ;-)).
msg339648 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-04-08 15:13
I would say that the natural output of mktime is indeed an integer, but I can't say off the top of my head what "compatibility" refers to here, so per the principle of Chesterton's fence, without more research or historical context I'd say the default should be *not* to make this change.

At the moment, I'm -0 on this, because "we should have designed the API this way in the first place" doesn't seem like a good enough reason to change the return type. I doubt it would be *terribly* onerous to the users to change this, so if there's some compelling reason for making it an integer I'm willing to be persuaded, but as it stands I don't think this is causing a huge amount of confusion or problems.

One other possible reason to mildly prefer no change is that if we were to change `struct_time` to accept a float for `tm_sec` (which I imagine might have more widespread consequences, admittedly), it wouldn't require a change to `mktime` to support sub-second offsets. Again, a compelling use case for an `int` return value could pretty easily overwhelm this objection.

> mktime() will not start losing precision before year 285,422,891 (which is quite far in the future ;-)).

I think we're witnessing the birth of the Y285M422K891 bug! winking face emoji
msg340346 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-16 14:24
I started a thread on python-dev:
https://mail.python.org/pipermail/python-dev/2019-April/157121.html
msg340354 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-16 16:18
https://mail.python.org/pipermail/python-dev/2019-April/157125.html

Guido van Rossum wrote:

"Consistency with C should not be the issue -- consistency between the time functions is important. (...) So let's drop the idea."

I close the issue.
History
Date User Action Args
2019-04-16 16:18:54vstinnersetstatus: open -> closed
resolution: not a bug
messages: + msg340354

stage: patch review -> resolved
2019-04-16 14:24:32vstinnersetmessages: + msg340346
2019-04-08 15:13:39p-gansslesetmessages: + msg339648
2019-04-08 13:04:13vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request12650
2019-04-08 12:54:17vstinnercreate