classification
Title: time.localtime(float("NaN")) does not raise a ValueError on all platforms
Type: behavior Stage: patch review
Components: Extension Modules, Interpreter Core Versions: Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: belopolsky Nosy List: Mariatta, belopolsky, gregory.p.smith, mark.dickinson, vstinner
Priority: normal Keywords: patch

Created on 2016-03-30 07:17 by gregory.p.smith, last changed 2019-01-10 16:53 by vstinner.

Pull Requests
URL Status Linked Edit
PR 3085 merged python-dev, 2017-08-13 22:00
PR 3467 merged python-dev, 2017-09-08 23:05
PR 11507 open vstinner, 2019-01-10 16:49
PR 11507 open vstinner, 2019-01-10 16:49
PR 11507 open vstinner, 2019-01-10 16:49
PR 11507 open vstinner, 2019-01-10 16:49
Messages (8)
msg262653 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2016-03-30 07:17
time.localtime(float("NaN")) raises a ValueError on x86_64 using the few compilers I have tested it with. (this makes sense)

>>> time.localtime(float("NaN"))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: (75, 'Value too large for defined data type')

On an arm and arm64 system, it does not and treats NaN as 0. (nonsense!)

>>> time.localtime(float("NaN"))
time.struct_time(tm_year=1970, tm_mon=1, tm_mday=1, tm_hour=0, tm_min=0, tm_sec=0, tm_wday=3, tm_yday=1, tm_isdst=0)

The root of this problem appears to be the (potentially questionable? I'll ask a C compiler person...) code in Python/pytime.c's (3.x) Modules/timemodule.c's (2.7) double to time_t conversion function.

I'm not sure what it does is supposed to be well defined behavior with NaN...

The easy fix is to add:

#include <math.h>

and

add || isnan(x) || isinf(x) to the check that raises a ValueError in

https://hg.python.org/cpython/file/4c903ceeb4d1/Python/pytime.c#l149 (3.x)
https://hg.python.org/cpython/file/2.7/Modules/timemodule.c#l102 (2.7)

Along with a relevant assertRaises(ValueError) unittest for NaN, inf and -inf in test_time.py.
msg300237 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2017-08-14 07:54
> potentially questionable? I'll ask a C compiler person...

Questionable indeed. Attempting to cast a NaN to an integer type results in undefined behaviour. Unfortunately, so does attempting to cast any double value that's outside the range represented by the `time_t` type, so the questionability extends beyond just the NaN handling.
msg301041 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-31 07:35
I like PR 3085 to raise explicitly a ValueError with an helpful error message.
msg301754 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-08 23:05
New changeset 829dacce4fca60fc3c3367980e75e21dfcdbe6be by Victor Stinner (Han Lee) in branch 'master':
bpo-26669: Fix nan arg value error in pytime.c (#3085)
https://github.com/python/cpython/commit/829dacce4fca60fc3c3367980e75e21dfcdbe6be
msg301755 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-09-08 23:09
For Python 2.7, you have at least to fix these 2 functions: parse_time_double_args(),  _PyTime_DoubleToTimet().
msg301760 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-09-09 04:53
New changeset a4baf1c543bca261c27e98ba296e42665f3cb872 by Mariatta (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-26669: Fix nan arg value error in pytime.c (GH-3085) (GH-3467) 
https://github.com/python/cpython/commit/a4baf1c543bca261c27e98ba296e42665f3cb872
msg301761 - (view) Author: Mariatta Wijaya (Mariatta) * (Python committer) Date: 2017-09-09 04:55
This has been backported to 3.6.
Is backport to 2.7 needed? If not we'll close the issue.
msg333403 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-01-10 16:53
I wrote PR 11507 to raise ValueError rather than OverflowError for -inf and +inf.
History
Date User Action Args
2019-01-10 16:53:59vstinnersetmessages: + msg333403
2019-01-10 16:49:45vstinnersetstage: backport needed -> patch review
pull_requests: + pull_request11052
2019-01-10 16:49:38vstinnersetstage: backport needed -> backport needed
pull_requests: + pull_request11051
2019-01-10 16:49:31vstinnersetstage: backport needed -> backport needed
pull_requests: + pull_request11050
2019-01-10 16:49:24vstinnersetstage: backport needed -> backport needed
pull_requests: + pull_request11049
2017-09-09 04:55:08Mariattasetmessages: + msg301761
stage: patch review -> backport needed
2017-09-09 04:53:07Mariattasetnosy: + Mariatta
messages: + msg301760
2017-09-08 23:09:27vstinnersetmessages: + msg301755
2017-09-08 23:05:14python-devsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request3460
2017-09-08 23:05:07vstinnersetmessages: + msg301754
2017-08-31 07:35:51vstinnersetnosy: + vstinner
messages: + msg301041
2017-08-14 07:54:14mark.dickinsonsetnosy: + mark.dickinson
messages: + msg300237
2017-08-13 22:00:41python-devsetpull_requests: + pull_request3127
2016-09-14 19:01:04belopolskysetassignee: belopolsky

type: behavior
nosy: + belopolsky
stage: needs patch
2016-03-30 07:17:23gregory.p.smithcreate