-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Inconsistent datetime.*.strftime()
year padding behavior across %y
, %Y
, %F
and %C
#122272
Comments
Hi, >>> date(909, 9, 9).strftime("%y")
'09' seems consistent with I can't find any mention of |
The doc has:
|
I'm not a contributor or a member of the Python community really so I don't know their rules etc. So take this as only my words. To me this seems like padding implementation problems for the libc standard, and if it's not in the documentation it shouldn't be assumed to be supported. And if you go around what is explicitly supported and the underlying C implementation is wrong it shouldn't be padded over, otherwise we could be discussing issues of wrong implementations of the C standards forever for every edge case where a developer decided to do it their own way and bypass what is explicitly supported by Python. And in the specific case of |
This argument is not internally consistent. Either you believe Python should just follow the C89 standard, in which case it shouldn't be padding things over, or it shouldn't, in which case there is no reason not to be consistent. The way things are right now is the worst outcome possible, it goes against the principle of the least surprise, it was changed very late in the beta testing phase of Python 3.13 and it actually breaks high-profile projects (I dare say Django is one).
I don't see how
This is a bold claim that you're making. "No bug reported" is hardly any evidence. Did you actually, say, grep Python projects from PyPI? AFAIR they had archives of "top N projects" for that purpose.
As the linked Django code points out, the problem has been reported back in 2011. |
cc @serhiy-storchaka and @erlend-aasland as reviewers too |
Yes, Django is important.
Yeah, misread some docs. It's different
Tone down your way of writing. This is a civilized place. I did not, I could have. Anyway, In My Oppinion It is not in the Python docs. It is not explicitly supported. And this code is the exact example of what I mean that users of a language should patch when they bypass what is explicitly stated to be supported and use platform specific hacks. It quite literally says it in the docstrings, this is a Django issue since they were patching over Python errors. Python fixed one of those errors, they should just patch their code plain and simple (In My Oppinion). def sanitize_strftime_format(fmt):
"""
Ensure that certain specifiers are correctly padded with leading zeros.
For years < 1000 specifiers %C, %F, %G, and %Y don't work as expected for
strftime provided by glibc on Linux as they don't pad the year or century
with leading zeros. Support for specifying the padding explicitly is
available, however, which can be used to fix this issue.
FreeBSD, macOS, and Windows do not support explicitly specifying the
padding, but return four digit years (with leading zeros) as expected.
This function checks whether the %Y produces a correctly padded string and,
if not, makes the following substitutions:
- %C → %02C
- %F → %010F
- %G → %04G
- %Y → %04Y
See https://bugs.python.org/issue13305 for more details.
"""
if datetime.date(1, 1, 1).strftime("%Y") == "0001":
return fmt
mapping = {"C": 2, "F": 10, "G": 4, "Y": 4}
return re.sub(
r"((?:^|[^%])(?:%%)*)%([CFGY])",
lambda m: r"%s%%0%s%s" % (m[1], mapping[m[2]], m[2]),
fmt,
) |
Can you clarify what you feel is uncivilized enough about that comment to be worth reprimanding someone for inappropriate behavior? Maybe I'm missing something. |
It looks like the change was also backported to 3.12 which means it will be breaking Django in a point release of an existing stable python version as well? This seems a lot more worrisome than a change in behavior "late in a beta cycle". |
Easy to say and then provide no contrary evidence for saying something that is not part of the Python documentation is often used. Just throw an argumentative fallacy out there.
Unnecessary sarcastic remark Or maybe I'm reading too much into it ?
I don't know what Python releases entail, and I don't know the extent and how you cover platform specific compatibility for issues like this, as I clearly stated that it's my opinion and I'm not privy to the Python community's ways. But yeah it is important to revert this change if this breaks things in production for 3.12 |
Yes, you're reading too much into it. He's using a single result that he uncovered as evidence that it happened at least once, and you responded to an existing report of people doing it, by saying people don't realistically do it. There is an existing report of breakage. It's not an "argumentative fallacy" to claim that the burden of proof is on people who want to make a change (in this case: who want to modify the padding behavior as present in a decade of released python versions, to behave differently in 3.13) to prove that the change is good -- the status quo never has the burden of proof. But that's really not my point. Calling someone uncivilized is something you should only do when you are quite certain the accusation is correct. Calling someone uncivilized and then being wrong with your accusation, is, well, "uncivilized behavior" and "false accusations". I would feel more comfortable if people don't accuse others of being uncivilized if they are concerned they may be reading too much into a phrase. I don't think "made use of a logical fallacy", on its own, constitutes sufficient evidence (whether it is or isn't a fallacy), and I don't think reading into a synonym for the phrase "for example" constitutes sufficient evidence either. |
It doesn't do so yet, because it was backported almost a month after the release of 3.12.4, so we have until 3.12.5 (roughly two weeks) before this hits django users in the wild (and also: distro packaging unittests for django). |
Very well put together, thank you. I shouldn't have used that word or assumed so much, sorry.
I see PR #121145 touches quite a few files and I've never contributed to this repo and don't have the time to get deep into it this week or weekend (no idea if you have a revert mechanism either). Do you know how to @eli-schwartz of do we need to wait for @blhsing ? |
Thank you for your report, @mgorny. This is definitely omission.
$ date -d 1-2-3 +%F
0001-02-03
$ date -d 1-2-3 +%C
00 What should we do practically?
cc @pablogsal, @Yhg1s |
@serhiy-storchaka thanks for the excellent summary of practical things to do. I have a small inclination to prefer postponing it to 3.14 since we are already past the very last beta ever -- the next cpython release will be release candidate 1, and is supposed to ideally be fully representative of the exact bytes installed by the final release. I think it's a bad idea to make further changes that need to be tested, but can only be tested in the release candidate, since at that point we kinda lost the chance to make any further changes without messing people up. If this hadn't been the very last beta then maybe it would be worth trying one last time to fix it for 3.13. But I would say that in between the final beta and the first release candidate, is when anything that's still an issue should be reverted for the sake of having the best release candidate we can. |
Many thanks to @mgorny for the report and to @serhiy-storchaka for the excellent summary and for taking on the task. Sorry for replying late, but I'll gladly fix it too if you haven't already started working on it. |
@serhiy-storchaka, will you be able to fix this by tomorrow's 3.13.0rc1? |
Sure. |
Go ahead! After reverting the changes in 3.12 and 3.13 we will not have a time pressure. |
Sure. I'll proceed with a fix then. |
@serhiy-storchaka Can you help review the PR? Thanks! Note that I don't think we should explicitly document the support for |
With the 3.13 & 3.12 reverts, this is not a release blocker any more. |
Bug report
Bug description:
#120713 changed the way
%y
and%Y
format works withstrftime()
to pad "short" years with leading zeros. However, the%F
and%C
formats were left alone, creating inconsistency:While I don't have a strong opinion about
%C
, the change to%Y
now means that%Y-%m-%d
no longer matches%F
.Notably, this change broke the logic in Django that assumed that if
%Y
returns a zero-padded year number, no padding needs to be done for all formats:/~https://github.com/django/django/blob/0e94f292cda632153f2b3d9a9037eb0141ae9c2e/django/utils/formats.py#L266-L273
I will report a bug there as well, but I think it would be beneficial to preserve consistency here.
Django bug: https://code.djangoproject.com/ticket/35630
CC @blhsing @martinvuyk
CPython versions tested on:
3.13, CPython main branch
Operating systems tested on:
Linux
Linked PRs
The text was updated successfully, but these errors were encountered: