gh-137165: Standardized non-zero-padded date formatting for datetime.strftime#139088
gh-137165: Standardized non-zero-padded date formatting for datetime.strftime#139088F0RRZZ wants to merge 19 commits intopython:mainfrom
datetime.strftime#139088Conversation
F0RRZZ
changed the title
datetime.strftimedatetime.strftime
|
!buildbot iOS |
This comment was marked as outdated.
This comment was marked as outdated.
|
The implementation looks reasonable, but this should also be covered in the documentation, along with the fact that the |
F0RRZZ
force-pushed
the
issue-137165
branch
from
67ec54d to
eb5c883
Compare
|
@mhsmith I adapted the tests for iOS and added documentation in the |
|
@StanFromIreland Made all changes |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
|
@mhsmith All done, but TSan seems to be stuck |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
@mhsmith Fixed, lets test |
mhsmith
added
the
🔨 test-with-refleak-buildbots
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
mhsmith
left a comment
mhsmith
left a comment
There was a problem hiding this comment.
I've started the refleaks tests, but I suggest you work out how to run them on your own machine rather than requiring a team member to rerun them on every commit. I don't know exactly how to do this, but it looks like it involves the -R option of python -m test. See the buildbot configuration for more details.
F0RRZZ
force-pushed
the
issue-137165
branch
from
a949bb5 to
0d53ced
Compare
|
@mhsmith Thanks! I'm running them on my machine, I just thought they needed to be run here as well to make sure everything works |
mhsmith
left a comment
mhsmith
left a comment
There was a problem hiding this comment.
Thanks, this looks much better. Just a few stylistic comments:
|
@mhsmith All done |
mhsmith
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @mhsmith for commit 9212a5a 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F139088%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
|
@pganssle Just a gentle ping on this PR review🙏 |
| def _make_dash_replacement(ch, timetuple): | ||
| fmt = '%' + ch | ||
| val = _time.strftime(fmt, timetuple) | ||
| return val.lstrip('0') or '0' |
There was a problem hiding this comment.
Will this not eat the dash for unsupported codes, e.g what will %-# do?
There was a problem hiding this comment.
I've added an explicit check to ensure that only valid format specifiers (as documented in Doc/library/datetime.rst) are passed on Windows and Android.
For other platforms I didn’t add this validation because they currently handle unsupported specifiers silently—just echoing them back. For example for "%-#":
- raises an error on Windows,
- returns "#" on macOS/FreeBSD,
- and returns "%-#" on Linux.
There was a problem hiding this comment.
So, removing one inconsistency you introduce another? I would expect them all to be the same as on linux, why are you raising an error?
There was a problem hiding this comment.
I've removed the ValueError and unified the behavior for unsupported specifiers like on Linux.
Now, for example, when entering "%-#" on all systems, "%-#" will be displayed.
4 participants
Previously, using non-zero-padded directives (e.g.,
%-d) instrftimecaused test failures on Windows and Android (x86_64), while other platforms worked as expected.This has been fixed by manually handling such specifiers on these platforms.
Corresponding tests have been added to strftime tests.
On Windows using non-zero-padded directives raised ValueError.
On Android (x86_64) dicrectives were not replaced:
