| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Most changes to Python require a NEWS entry. Please add it using the blurb_it web app or the blurb command-line tool. |
Sorry, something went wrong.
Sorry, something went wrong.
There was a problem hiding this comment.
Very elegant, and the code LGTM. But would you be so kind as to add a few unit tests?
Sorry, something went wrong.
|
@gvanrossum thanks. i even tried to start with unit tests, but it looked hard to fit in the existing test code. but i'll try again... :-) |
Sorry, something went wrong.
|
Here are the tests for %Z: https://github.com/python/cpython/blob/main/Lib/test/datetimetester.py#L1453 You could add another method, create the datetime instances that you need, then compare method calls with expected outputs. |
Sorry, something went wrong.
datetime.isoformat generates the tzoffset with colons, but there was no format code to make strftime output the same format. for simplicity and consistency the %:z formatting behaves mostly as %z, with the exception of adding colons. this includes the dynamic behaviour of adding seconds and microseconds only when needed (when not 0). this fixes the still open "generate" part of this issue: #69142
|
@merwok thanks. @gvanrossum first attempt was very elegant / short, but incorrect:
|
Sorry, something went wrong.
There was a problem hiding this comment.
I haven't tried to look into why the tests still fail. You know you can run tests locally right?
Also, please don't force-push (which I think you've done) once the review is under way. We'll squash-merge in the end anyway.
Sorry, something went wrong.
|
Please avoid force pushes for Python PRs: https://devguide.python.org/getting-started/pull-request-lifecycle/index.html |
Sorry, something went wrong.
|
Of course I ran the tests locally before pushing (but maybe in an unusual way), so there is this interesting finding (on macOS 12, in case it matters): % ./python.exe Lib/test/datetimetester.py -v # all works (including the test fails seen in next command) % ./python.exe -m test -v test_datetime # test_datetime fails |
Sorry, something went wrong.
There was a problem hiding this comment.
There seems to be still something failing. Do you need help figuring that out?
Sorry, something went wrong.
(as seen for the other replacements)
|
@gvanrossum yes, i could not find the cause for the strange behaviour yet. Update: Looks like _Fast tests are ok vs _Pure tests are broken. Ah, just found Lib/datetime.py is the pure python implementation that also needs adapting. |
Sorry, something went wrong.
|
That patchcheck automation is not very helpful. It does not show errors, nor line numbers, just recommends to run make patchcheck locally, which is broken for me: tw@mba2020 cpython % make patchcheck Checked 109 modules (30 built-in, 77 shared, 2 n/a on macosx-12.5-arm64, 0 disabled, 0 missing, 0 failed on import) ./python.exe ./Tools/scripts/patchcheck.py Enter passphrase for key '...': upstream/main Getting the list of files that have been added/changed ... fatal: ambiguous argument 'upstream/main': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' error running git diff --name-status upstream/main make: *** [patchcheck] Error 1 |
Sorry, something went wrong.
|
it works for me, and I do have the recommended setup with an upstream origin for this repo (origin being my fork) |
Sorry, something went wrong.
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
There was a problem hiding this comment.
Getting very close. I have some nits to pick about the docs.
@kumaraditya303 Have you reviewed datetime.py and is it to your liking?
Sorry, something went wrong.
There was a problem hiding this comment.
Have you reviewed datetime.py and is it to your liking?
LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
Yes! I'll merge. Thanks TW for the PR and thanks Kumar for all the review work you did.
Sorry, something went wrong.
|
BTW, will we have to wait some years for this until 3.12 becomes minimum requirement or can this go into older pythons also? |
Sorry, something went wrong.
Alas, this looks like a new feature to me so it won't be backported. |
Sorry, something went wrong.
|
OK, pity. OTOH, even if it were backported, it would only go into 3.11.latest, so would not help if somebody has 3.11.lessthanthat. |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot s390x Debian 3.x has failed when building commit 023c51d. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/49/builds/3543 Failed tests:
Summary of the results of the build (if available): == Tests result: FAILURE then ENV CHANGED == 420 tests OK. 10 slowest tests:
1 test altered the execution environment: 14 tests skipped: 1 re-run test: Total duration: 19 min 40 sec Click to see traceback logsremote: Enumerating objects: 16, done.
remote: Counting objects: 6% (1/15)
remote: Counting objects: 13% (2/15)
remote: Counting objects: 20% (3/15)
remote: Counting objects: 26% (4/15)
remote: Counting objects: 33% (5/15)
remote: Counting objects: 40% (6/15)
remote: Counting objects: 46% (7/15)
remote: Counting objects: 53% (8/15)
remote: Counting objects: 60% (9/15)
remote: Counting objects: 66% (10/15)
remote: Counting objects: 73% (11/15)
remote: Counting objects: 80% (12/15)
remote: Counting objects: 86% (13/15)
remote: Counting objects: 93% (14/15)
remote: Counting objects: 100% (15/15)
remote: Counting objects: 100% (15/15), done.
remote: Compressing objects: 7% (1/13)
remote: Compressing objects: 15% (2/13)
remote: Compressing objects: 23% (3/13)
remote: Compressing objects: 30% (4/13)
remote: Compressing objects: 38% (5/13)
remote: Compressing objects: 46% (6/13)
remote: Compressing objects: 53% (7/13)
remote: Compressing objects: 61% (8/13)
remote: Compressing objects: 69% (9/13)
remote: Compressing objects: 76% (10/13)
remote: Compressing objects: 84% (11/13)
remote: Compressing objects: 92% (12/13)
remote: Compressing objects: 100% (13/13)
remote: Compressing objects: 100% (13/13), done.
remote: Total 16 (delta 2), reused 3 (delta 2), pack-reused 1
From https://github.com/python/cpython
* branch main -> FETCH_HEAD
Note: switching to '023c51d9d8e2fd91069eea2bf12e373f1c71e9d2'.
You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.
If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:
git switch -c <new-branch-name>
Or undo this operation with:
git switch -
Turn off this advice by setting config variable advice.detachedHead to false
HEAD is now at 023c51d9d8 gh-69142: add %:z strftime format code (gh-95983)
Switched to and reset branch 'main'
make: *** [Makefile:1865: buildbottest] Error 3 |
Sorry, something went wrong.
|
I don't see how this change could break test_multiprocessing_forkserver. From the logs it appears that the first time this test ran it exceeded the 15 minute time limit. The second time it ran fine. Let's ignore this unless the buildbot police get involved. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
datetime.isoformat generates the tzoffset with colons, but there
was no format code to make strftime output the same format.
for simplicity and consistency the %:z formatting behaves mostly
as %z, with the exception of adding colons. this includes the
dynamic behaviour of adding seconds and microseconds only when
needed (when not 0).
this fixes the still open "generate" part of this issue:
#69142