GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — bpo-30155: Add macros to get tzinfo from datetime instances by ZackerySpytz · Pull Request #21633 · python/cpython

Conversation

@ZackerySpytz

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

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Add PyDateTime_DATE_GET_TZINFO() and PyDateTime_TIME_GET_TZINFO()
macros.

https://bugs.python.org/issue30155

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ZackerySpytz
ZackerySpytz requested review from abalkin and pganssle as code owners July 27, 2020 01:44
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Jul 27, 2020
@bedevere-bot bedevere-bot added the awaiting review label Jul 27, 2020
kk456852
kk456852 approved these changes Jul 27, 2020
View reviewed changes
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Jul 27, 2020
pganssle
pganssle requested changes Jul 27, 2020
View reviewed changes

@pganssle pganssle left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Modules/_zoneinfo.c Outdated

@pganssle pganssle Jul 27, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@ZackerySpytz ZackerySpytz Jul 29, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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. :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/datetimetester.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/datetimetester.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting changes and removed awaiting core review labels Jul 27, 2020
@bedevere-bot

bedevere-bot commented Jul 27, 2020

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ZackerySpytz

ZackerySpytz commented Jul 29, 2020

Copy link
Copy Markdown
Contributor Author

Thanks, @pganssle.

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Jul 29, 2020
@bedevere-bot

bedevere-bot commented Jul 29, 2020

Copy link
Copy Markdown

Thanks for making the requested changes!

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from pganssle July 29, 2020 03:09
@sebukoleth

sebukoleth commented Sep 23, 2020

Copy link
Copy Markdown

Can we get this change in, please?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
pganssle
pganssle approved these changes Sep 23, 2020
View reviewed changes

@pganssle pganssle left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Sep 23, 2020
ZackerySpytz added 2 commits September 23, 2020 12:55
@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 force-pushed the bpo-30155-tzinfo-macros branch from b25db0e to 83db28d Compare September 23, 2020 16:56
@pganssle
pganssle merged commit 2e4dd33 into python:master Sep 23, 2020
@bedevere-bot bedevere-bot removed the awaiting merge label 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

sebukoleth commented Sep 24, 2020

Copy link
Copy Markdown

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@pganssle

pganssle commented Sep 24, 2020

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
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 @xzy3-cdc
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
@abalkin abalkin Awaiting requested review from abalkin
+1 more reviewer
@kk456852 kk456852 kk456852 approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@ZackerySpytz @bedevere-bot @sebukoleth @pganssle @kk456852 @the-knights-who-say-ni
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.