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.

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

Add ability to get tzinfo from a datetime instance in C API #74341

Closed
atuining mannequin opened this issue Apr 24, 2017 · 7 comments
Closed

Add ability to get tzinfo from a datetime instance in C API #74341

atuining mannequin opened this issue Apr 24, 2017 · 7 comments
Labels
3.10 only security fixes stdlib Python modules in the Lib dir topic-C-API type-feature A feature request or enhancement

Comments

@atuining
Copy link
Mannequin

atuining mannequin commented Apr 24, 2017

BPO 30155
Nosy @abalkin, @pitrou, @dlenski, @pganssle, @ZackerySpytz
PRs
  • bpo-30155: Add macros to get tzinfo from datetime instances #21633
    		•
    Dependencies
  • bpo-10381: Add timezone support to datetime C API
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2020-10-21.19:51:20.376>
created_at = <Date 2017-04-24.15:27:26.203>
labels = ['expert-C-API', 'type-feature', 'library', '3.10']
title = 'Add ability to get tzinfo from a datetime instance in C API'
updated_at = <Date 2020-10-21.19:51:20.376>
user = 'https://bugs.python.org/atuining'

    bugs.python.org fields:

    activity = <Date 2020-10-21.19:51:20.376>
actor = 'p-ganssle'
assignee = 'none'
closed = True
closed_date = <Date 2020-10-21.19:51:20.376>
closer = 'p-ganssle'
components = ['Library (Lib)', 'C API']
creation = <Date 2017-04-24.15:27:26.203>
creator = 'atuining'
dependencies = ['10381']
files = []
hgrepos = []
issue_num = 30155
keywords = ['patch']
message_count = 7.0
messages = ['292230', '321112', '361722', '361737', '374009', '374354', '377417']
nosy_count = 6.0
nosy_names = ['atuining', 'belopolsky', 'pitrou', 'dlenski', 'p-ganssle', 'ZackerySpytz']
pr_nums = ['21633']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30155'
versions = ['Python 3.10']
    @atuining
    Copy link
    Mannequin Author

    atuining mannequin commented Apr 24, 2017

    Right now there is no documented way to create a datetime instance with a tzinfo instance. The documented macros all hard code the value Py_None for the tzinfo parameter. Using the PyObject_Call() method instead of the macro for creating a datetime instance is ~5x slower.

    In addition, there is no macro or method for getting the tzinfo from an existing datetime instance. Perhaps creating DATE_GET_TZINFO and TIME_GET_TZINFO would be acceptable?

    The enhancement 10381 (http://bugs.python.org/issue10381) would also be needed.

    I can provide a GitHub PR if that would be helpful. I first want to make sure that such an effort would be appreciated!

    @atuining atuining mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Apr 24, 2017
    @pganssle
    Copy link
    Member

    pganssle commented Jul 5, 2018

    Hmm. I never noticed this. In the past I have used the (undocumented) PyDateTimeAPI struct, which the official macros wrap. I'm not sure how official that struct is considering it doesn't show up in the documentation.

    I agree that it should be possible to construct TZ-aware datetimes using the official C API, so I guess the question is whether to add a bunch more macros or document the existing struct.

    @atuining
    Copy link
    Mannequin Author

    atuining mannequin commented Feb 10, 2020

    Any progress on this?

    @pganssle
    Copy link
    Member

    So this bug is asking for two things:

    1. An official accessor for the tzinfo component of an existing datetime, which I think is very reasonable in light of the fact that there are official accessors for all the other components of a datetime.

    2. An official constructor for a timezone-aware datetime, which I think basically exists in the form of PyDatetime_CAPI->PyDateTimeAPI->DateTime_FromDateAndTime / ->DateTime_FromDateAndTimeAndFold, and we just need to document it. I think this is basically a separate issue, and I have opened bpo-39604 to track it.

    I'm going to rename this bug to focus only on issue #1. I think we can accept a PR adding two new macros. I would suggest calling them:

    • PyDateTime_DATE_GET_TZINFO
    • PyDateTime_TIME_GET_TZINFO

    Please make sure to add tests to any PR you make. See the CapiTest case (

    cpython/Lib/test/datetimetester.py

    Line 5914 in d68e0a8

    class CapiTest(unittest.TestCase):
    ) for examples. You may want to look at the git blame for a few of those tests to see the PRs that they were added in, since part of the tests are defined in a C file.

    (As an aside: I don't love that the accessor methods are not available on the struct, since all the "macro-only" code needs to be re-implemented in all other-language bindings. Since the accessors are already all macro-only, though, might as well keep with the tradition for now :P)

    @pganssle pganssle added the 3.9 only security fixes label Feb 10, 2020
    @pganssle pganssle changed the title Add ability to get/set tzinfo on datetime instances in C API Add ability to get tzinfo from a datetime instance in C API Feb 10, 2020
    @pitrou
    Copy link
    Member

    pitrou commented Jul 20, 2020

    I'm going to rename this bug to focus only on issue #1. I think we can accept a PR adding two new macros. I would suggest calling them:

    • PyDateTime_DATE_GET_TZINFO
    • PyDateTime_TIME_GET_TZINFO

    +1 for this.

    @ZackerySpytz
    Copy link
    Mannequin

    ZackerySpytz mannequin commented Jul 27, 2020

    I have created PR 21633 to add these macros. Please consider taking a look.

    @ZackerySpytz ZackerySpytz mannequin added 3.10 only security fixes and removed 3.9 only security fixes labels Jul 27, 2020
    @pganssle
    Copy link
    Member

    New changeset 2e4dd33 by Zackery Spytz in branch 'master':
    bpo-30155: Add macros to get tzinfo from datetime instances (GH-21633)
    2e4dd33

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.10 only security fixes stdlib Python modules in the Lib dir topic-C-API type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @vstinner @pganssle @pitrou