Repository navigation
datetime library doesn't support valid ISO-8601 alternative for midnight #102450
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
on Mar 5, 2023 From the referenced wikipedia page:
As of ISO 8601-1:2019/Amd 1:2022, midnight may be referred to as "00:00:00", corresponding to the instant at the beginning of a calendar day; or "24:00:00", corresponding to the instant at the end of a calendar day.[1] ISO 8601-1:2019 as originally published removed "24:00" as a representation for the end of day although it was permitted in earlier versions of the standard.
Looking at the datetime commit log, I am pretty sure that 24:00:00 support was not removed after the 2019 version but was never there. It is obviously a nuisance.
While the doc for datetime.datetime explicitly disclaims such support ("0 <= hour < 24"), the doc for datetime.datetime.fromisoformat does not. It says "Return a datetime corresponding to a date_string in any valid ISO 8601 format, with the following exceptions:" and lists 4 exceptions. To me, the bug here is the omission of the pretty clearly intended '240000' exception. The fix would be to add the 5th exception to the doc.
The request to add support would then be a feature addition. I have no opinion, but think it less likely to be accepted than rejected.
Reacted by TizzySaurusSee also #54636
I think this actually is a bug. The original intention was that
fromisoformatwas supposed to support parsing any valid ISO-8601 string that can be represented by adatetime.datetimeobject (plus certain expansions so that all possible outputs ofdatetime.isoformat()are covered). We decided not to support a few things like fractional minutes and fractional hours because we believe that these formats are so rarely used that if you encounter them in the wild, it's more likely that you have encountered a bug than something real that you want to parse.Support for 24 as an alternative to midnight doesn't really fit into that category, and can be useful in certain situations, so I'd be happy to see a PR for this.
Reacted by Mark DickinsonI think this actually is a bug. ...I'd be happy to see a PR for this.
I agree with you that this is something I feel is a bug and I'd like to see added, even with the points mentioned by @terryjreedy. To that end, I'd be interested in writing the PR for this.
Code wise, is it just the
_check_time_fieldsfunction and thecheck_time_argsC function that would need to be updated?I suppose documentation would also need to be updated, but I'm not sure how to do this.
It seems the queries in #54636 were addressed, but just to confirm, I'd expect
datetime(2023, 1, 1, 24)to be parsed equal todatetime(2023, 1, 2, 0)(and thus.daywould be2,.hourwould be0, etc. -- the.hourproperty of a created datetime object will still never be 24), and inclusion of smaller units not equal to zero (such asdatetime(2023, 1, 1, 24, 0, 0, 567890)) is invalid and will error.What about
datetime.time? There the situation is different -time(0, 0, 0)should not be equal totime(24, 0, 0). And I doubtdatetime.timecan be extended at this point to supporttime(24, 0, 0)as a distinct value. Sodatetime.time.fromisoformatwould have to keep rejecting24:00:00, which would be inconsistent withdatetime.datetime.fromisoformatif this proposal is accepted.Code wise, is it just the
_check_time_fieldsfunction and thecheck_time_argsC function that would need to be updated?No, we shouldn't make it so that
datetime.datetimeordatetime.timeaccept "24" as a valid value, the adjustment should happen in the parsing stage. This is how it's done indateutil.parser.isoparse.I think you want to update
datetime_fromisoformatin the C code andfromisoformatin the Python code. The equivalent functions indatetime.timejust need to be updated so that when you get24:00it is rendered astime(0, 0, 0).You will also want to add test cases to
datetimetester.py. In addition to test cases like"2023-05-14T24:00"rendering todatetime(2023, 5, 15), you also want to make sure2023-05-14T24:01still raises an exception.What about
datetime.time? There the situation is different -time(0, 0, 0)should not be equal totime(24, 0, 0). And I doubtdatetime.timecan be extended at this point to supporttime(24, 0, 0)as a distinct value. Sodatetime.time.fromisoformatwould have to keep rejecting24:00:00, which would be inconsistent withdatetime.datetime.fromisoformatif this proposal is accepted.This proposal should have no effect on the
datetimeandtimeprimary constructors. If ISO-8601 says thatT24:00is midnight (and I think it does), thendatetime.time.fromisoformat("24:00")should returndatetime.time(0, 0, 0).Reacted by TizzySaurusAccording to the Wikipedia quote above, time
00:00is beginning midnight, and24:00is the next day's midnight.Just to explain where I ran into it, I work on a system which defines "parts of day" like this (for example):
- Night 00:00 - 05:00
- Morning 05:00 - 11:00
- Noon: 11:00 - 17:00
- Evening 17:00 - 24:00
This is in a PostgreSQL database using the
timetype. The ranges are [inclusive, exclusive). If 24:00 becomes 00:00, the Evening range becomestime(17, 0), time(0, 0)which doesn't work.So the two midnights look like different things to me, I don't think
24:00should parse totime(0).This is in a PostgreSQL database using the
timetype. The ranges are [inclusive, exclusive). If 24:00 becomes 00:00, the Evening range becomestime(17, 0), time(0, 0)which doesn't work.So the two midnights look like different things to me, I don't think
24:00should parse totime(0).I mean, the alternative is that it doesn't parse at all and you simply cannot represent the last interval, so it doesn't work either way. At least if the parse works, you have the option to find a different way to represent that interval.
timerepresents a time of day, and there's only one midnight per day. My copy of ISO-8601 says that 24:00 represents the end of the day and there's no suggestion that this is only valid as part of adatetime, so I think we're supposed to parse it astime(0), even though it's true that this is necessarily a lossy conversion.No, we shouldn't make it so that datetime.datetime or datetime.time accept "24" as a valid value, the adjustment should happen in the parsing stage.
This proposal should have no effect on the
datetimeandtimeprimary constructors.Does this mean you're saying that
datetime.datetime.fromisoformat("2023-01-02 24:00:00")should be valid, butdatetime.datetime(2023, 1, 2, 24, 0, 0)should be invalid? That wasn't strictly what I was going for with this issue, but I guess it is strictly more accurate (since this is an iso thing). Although it does seem odd to have it valid infromisoformatand the equivalent constructor be invalid.I was originally envisioning something along the lines of
def the_datetime_primary_constructor(year, month, day, hour, second, microsecond): if (hour == 24): if (second == microsecond == 0): hour = 0 day += 1 # with the correct handling for "rollover", i.e. adjusting month/year as necessary. else: raise ValueError("For hour to be 24, second and microsecond must be 0.") ...
This means that
hours=24would be valid in the constructor, but.hoursof the returned object would still return0Does this mean you're saying that
datetime.datetime.fromisoformat("2023-01-02 24:00:00")should be valid, butdatetime.datetime(2023, 1, 2, 24, 0, 0)should be invalid? That wasn't strictly what I was going for with this issue, but I guess it is strictly more accurate (since this is an iso thing). Although it does seem odd to have it valid infromisoformatand the equivalent constructor be invalid.Yes, I'm saying precisely this. The main constructor and underlying data model does not have anything to do with ISO 8601. Consider that
2023-W01-1is also a valid ISO 8601 representation fordatetime(2023, 1, 2). If there's a case to be made for accepting "24" for the "hour" component ofdatetime.datetime, it should be made independent of whether or not there's a corresponding ISO 8601 format.Also, I will note my bias here: ISO 8601 is an annoying and awful spec. It's proprietary and not even freely available. It's ridiculously convoluted and so expansive that no parser I know of implements the whole spec. Despite the byzantine complexity, it is also vague about things like what the date-time separator can be or how many digits can come after the fraction separator and it has no provision for things like fractional offsets. RFC3339 tries to tame the complexity but is only acceptable for representing absolute times. No one understands the ISO week calendar, either. Everyone thinks they like ISO 8601 because YYYY-MM-DD is a good way to represent dates, and people have converged on that, but if I could I would burn ISO 8601 to the ground and start over (sure we'd probably also get some weird camel of a format designed by committee, but at least if we did it today it would be in the public domain).
Needless to say, the fact that ISO 8601 does some weird thing is not a good argument for importing that particular abstraction. It's probably a good argument for including it in
fromisoformat, but that's the extent of it.... It's probably a good argument for including it in fromisoformat, but that's the extent of it.
Yeah, fair enough. I'll go ahead and work on a PR just for the change to
fromisoformat(and relevant tests etc.) then 👍I'm having some issues with "building" changes that I'm making to the python files. I found the contributing guide section on building, and followed that, but my changes to the
Lib/_pydatetime.pyfile aren't being reflected in the outputtedpython.exe(I'm on MacOS).Steps I'm doing:
- Add
print("Hello from datetime.datetime.fromisoformat()")above L1855 - Run
./configure --with-pydebug && make -s -j && ./python.exe - Enter the following code into the repl:
>>> import datetime >>> datetime.datetime.fromisoformat("2022-01-01 00:00:00")
The above steps don't appear to result in the
printstatement being executed, as I'd expect. I've also triedreturn cls(2000, 01, 01, 0, 0, 0, 0)to see if outputs are somehow blocked, but that doesn't work either.According to the note in the dev guide, I shouldn't need to build for changes to python code, but figured I'd try that when it didn't seem to work.
Can I please get some assistance?
- Add
This isn't the best place to get help with this, probably something on discuss.python.org would be better.
But as a hint, the code is probably in Modules/_datetimemodule.c.
Reacted by TizzySaurusBut as a hint, the code is probably in Modules/_datetimemodule.c.
To be clear, the code is in both,
datetimeis a PEP 399 module.@TizzySaurus Feel free to make a draft PR and ping me on it.
Reacted by TizzySaurusYes, @pganssle is correct. Thanks for the PEP reference, I couldn't find it quickly.
- addedstdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directory
on Nov 26, 2023 - added a commit that references this issue
on Sep 25, 2024 - added a commit that references this issue
on Sep 26, 2024 It will be in 3.14.
Reacted by TizzySaurus
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Bug report
According to ISO-8601, a time of
24:00on a given date is a valid alternative to00:00of the following date, however Python does not support this, raising the following error when attempted:ValueError: hour must be in 0..23.This bug can be seen from multiple scenarios, specifically anything that internally calls the
_check_time_fieldsfunction, such as the following:The fix for this is relatively simple: have an explicit check within
_check_time_fieldsfor the scenario wherehour == 24 and minute == 0 and second == 0 and microsecond == 0, or more conciselyhour == 24 and not any((minute, second, microsecond)), and in this scenario increase the day by one (adjusting the week/month/year as necessary) and set the hour to 0.I imagine the
check_time_argsC function would also have to be updated.Your environment
Linked PRs
fromisoformat()calls. #105856