Skip to content
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.

Sign up for GitHub

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

Jump to bottom

bpo-30155: Add macros to get tzinfo from datetime instances #21633

Merged
merged 2 commits into from Sep 23, 2020
Merged

bpo-30155: Add macros to get tzinfo from datetime instances #21633

merged 2 commits into from Sep 23, 2020

Conversation

ZackerySpytz
Copy link
Contributor

@ZackerySpytz ZackerySpytz commented Jul 27, 2020 •
edited by bedevere-bot

Add PyDateTime_DATE_GET_TZINFO() and PyDateTime_TIME_GET_TZINFO()
macros.

https://bugs.python.org/issue30155

pganssle
pganssle requested changes Jul 27, 2020
View reviewed changes
Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. Some minor comments, but overall I'm really happy with this PR. Thanks for putting it together!

Modules/_zoneinfo.c
#define HASTZINFO(p) (((_PyDateTime_BaseTZInfo *)(p))->hastzinfo)
#define GET_DT_TZINFO(p) \
(HASTZINFO(p) ? ((PyDateTime_DateTime *)(p))->tzinfo : Py_None)
#define GET_DT_TZINFO PyDateTime_DATE_GET_TZINFO
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively we can replace all the instances of GET_DT_TZINFO with PyDateTime_DATE_GET_TZINFO. I think either way works, because in the backport I'm either going to do:

#ifdef PyDateTime_DATE_GET_TZINFO
#define GET_DT_TZINFO PyDateTime_DATE_GET_TZINFO
#else
#define HASTZINFO ...

or I'll do this:

#ifndef PyDateTime_DATE_GET_TZINFO
#define _PyDateTime_HAS_TZINFO ...
#define PyDateTime_DATE_GET_TZINFO (_PyDateTime_HAS_TZINFO(p) ? ...

Then the rest of the file will be the same.

I was originally liking the first one, but upon further reflection, I like the second one. What do you think? (It's also OK to say that you like the second one more but you want to keep it out of scope of this PR).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the second one more, but I think it is out of scope for this PR. :)

Lib/test/datetimetester.py Outdated Show resolved Hide resolved
Lib/test/datetimetester.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@ZackerySpytz
Copy link
Contributor Author

Thanks, @pganssle.

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@pganssle: please review the changes made to this pull request.

@sebukoleth
Copy link

Can we get this change in, please?

pganssle
pganssle approved these changes Sep 23, 2020
View reviewed changes
Copy link
Member

@pganssle pganssle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I completely forgot about this PR. LGTM!

@ZackerySpytz @pganssle
bpo-30155: Add macros to get tzinfo from datetime instances
e018229 
Add PyDateTime_DATE_GET_TZINFO() and PyDateTime_TIME_GET_TZINFO()
macros.
@ZackerySpytz @pganssle
Use timezone.utc
83db28d 
Use assertIs().
@pganssle pganssle merged commit 2e4dd33 into python:master Sep 23, 2020
shihai1991 added a commit to shihai1991/cpython that referenced this pull request Sep 24, 2020
@shihai1991
Merge remote-tracking branch 'origin/master' into bpo_39337_new_3
e29ec44 
* origin/master: (27 commits)
  bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388)
  bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387)
  bpo-41833: threading.Thread now uses the target name (pythonGH-22357)
  bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633)
  bpo-33822: Update IDLE section of What's New 3.8 (pythonGH-22383)
  bpo-41844: Add IDLE section to What's New 3.9 (GN-22382)
  bpo-41841: Prepare IDLE News for 3.10 (pythonGH-22379)
  bpo-37779 : Add information about the overriding behavior of ConfigParser.read (pythonGH-15177)
  bpo-40170: Use inline _PyType_HasFeature() function (pythonGH-22375)
  bpo-40941: Fix stackdepth compiler warnings (pythonGH-22377)
  bpo-40941: Fix fold_tuple_on_constants() compiler warnings (pythonGH-22378)
  bpo-40521: Fix PyUnicode_InternInPlace() (pythonGH-22376)
  bpo-41834: Remove _Py_CheckRecursionLimit variable (pythonGH-22359)
  bpo-1635741, unicodedata: add ucd_type parameter to UCD_Check() macro (pythonGH-22328)
  bpo-1635741: Port _lsprof extension to multi-phase init (PEP 489) (pythonGH-22220)
  bpo-41513: Improve order of adding fractional values. Improve variable names. (pythonGH-22368)
  bpo-41816: `StrEnum.__str__` is `str.__str__` (pythonGH-22362)
  bpo-35764: Rewrite the IDLE Calltips doc section  (pythonGH-22363)
  bpo-41810: Reintroduce `types.EllipsisType`, `.NoneType` & `.NotImplementedType` (pythonGH-22336)
  bpo-41602: raise SIGINT exit code on KeyboardInterrupt from pymain_run_module (python#21956)
  ...
@sebukoleth
Copy link

Requesting backport of this PR to versions from 3.6 onward. Please let me know if there is a different place/way to request backport.

@pganssle
Copy link
Member

@sebukoleth This changes the C API, features like this are not backported to earlier point releases, which means that this will be a 3.10+ only feature.

On the bright side, though, it would be inappropriate to change the structure of the PyDateTime_DateTime object in a patch (i.e. bugfix) release, so you can much more comfortably rely on the implementation detail and just manually create your own macro like this:

#ifndef PyDateTime_DATE_GET_TZINFO
#define PyDateTime_DATE_GET_TZINFO(o)      (_PyDateTime_HAS_TZINFO(o) ? \
    ((PyDateTime_DateTime *)(o))->tzinfo : Py_None)
#endif

Or something equivalent to that.

shihai1991 added a commit to shihai1991/cpython that referenced this pull request Sep 29, 2020
@shihai1991
Merge remote-tracking branch 'origin/master' into bpo_1635741_1
e3b9365 
* origin/master: (113 commits)
  bpo-41773: Raise exception for non-finite weights in random.choices().  (pythonGH-22441)
  bpo-41873: Add vectorcall for float() (pythonGH-22432)
  bpo-41861: Convert _sqlite3 PrepareProtocolType to heap type (pythonGH-22428)
  bpo-41842: Add codecs.unregister() function (pythonGH-22360)
  bpo-41875: Use __builtin_unreachable when possible (pythonGH-22433)
  bpo-40105: ZipFile truncate in append mode with shorter comment (pythonGH-19337)
  bpo-41870: Use PEP 590 vectorcall to speed up bool()  (pythonGH-22427)
  [doc] Leverage the fact that the actual types can now be indexed for typing (pythonGH-22340)
  bpo-41861: Convert _sqlite3 cache and node static types to heap types (pythonGH-22417)
  bpo-41858: Clarify line in optparse doc (pythonGH-22407)
  Revert "Fix all Python Cookbook links (python#22205)" (pythonGH-22424)
  bpo-1635741: Port _bisect module to multi-phase init (pythonGH-22415)
  bpo-41428: Fix compiler warning in unionobject.c (pythonGH-22416)
  Fix logging error message (pythonGH-22410)
  bpo-39934: Account for control blocks in 'except' in compiler. (pythonGH-22395)
  bpo-41775: Make 'IDLE Shell' the shell title  (python#22399)
  bpo-41428: Fix compiler warnings in unionobject.c (pythonGH-22388)
  bpo-41654: Fix compiler warning in MemoryError_dealloc() (pythonGH-22387)
  bpo-41833: threading.Thread now uses the target name (pythonGH-22357)
  bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH-21633)
  ...
xzy3 pushed a commit to xzy3/cpython that referenced this pull request Oct 18, 2020
@ZackerySpytz
bpo-30155: Add macros to get tzinfo from datetime instances (pythonGH…
7642051 
…-21633)

Add PyDateTime_DATE_GET_TZINFO() and PyDateTime_TIME_GET_TZINFO()
macros.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pganssle pganssle pganssle approved these changes

@kk456852 kk456852 kk456852 approved these changes

@abalkin abalkin Awaiting requested review from abalkin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@ZackerySpytz @bedevere-bot @sebukoleth @pganssle @kk456852 @the-knights-who-say-ni