Skip to content

Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs #110850

Closed
Closed
Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs#110850
Labels
3.13only security fixesrelease-blockertopic-C-APItype-featureA feature request or enhancement

Description

@ryancdotorg
ryancdotorg
opened on Oct 14, 2023

Feature or enhancement

Proposal:

#106316 #106317 removed pytime.h, and I was using some things

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

Activity

  1. ryancdotorg
    added
    type-featureA feature request or enhancement
    on Oct 14, 2023
  2. hugovk
    changed the title [-]Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public apis[/-] [+]Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs[/+] on Oct 14, 2023

  3. hugovk commented on Oct 14, 2023

    @hugovk
    hugovk
    on Oct 14, 2023
    Member

    Please could you say something about your use case?

    Why do you need these functions?

    Have you tried alternative APIs to achieve the same result?

    cc @vstinner

  4. hugovk
    added
    topic-C-API
    on Oct 14, 2023

  5. ryancdotorg commented on Oct 14, 2023

    @ryancdotorg
    ryancdotorg
    on Oct 14, 2023
    Author

    I have a C/C++ module that was using them which I wrote a couple years ago. It broke.

    I actually dug into what I was doing, and it turns out it was basically just (error checking omitted):

    _PyTime_t tp;
struct timeval tv;
get_timestamp(&tv, ...);
_PyTime_FromTimeval(&tp, &tv);
double ts = _PyTime_AsSecondsDouble(tp);

    and I was able to more sensibly replace it with

    double tv_frac = tv.tv_usec / 1e6;
struct timeval tv;
get_timestamp(&tv, ...);
double tv_frac = tv.tv_usec / 1e6;
double ts = tv.tv_sec + tv_frac;

    It seems I just didn't want to bother figuring out the safest way to do the conversion.

    This can be closed.

  6. hugovk
    closed this as not plannedon Oct 14, 2023

  7. vstinner commented on Oct 14, 2023

    @vstinner
    vstinner
    on Oct 14, 2023
    Member

    I'm open to make some _Py_Time functions public, but apparently here, there wasn't a strong need for that. Maybe it can wait. Thanks for the report anyway.

  8. scoder commented on Oct 30, 2023

    @scoder
    scoder
    on Oct 30, 2023
    Contributor

    We actually expose that API in Cython's cpython/time.pxd. Please move it back to where it was.

  9. scoder
    reopened this on Oct 30, 2023

  10. vstinner commented on Oct 30, 2023

    @vstinner
    vstinner
    on Oct 30, 2023
    Member

    We actually expose that API in Cython's cpython/time.pxd. Please move it back to where it was.

    This file uses the following APIs:

    • _PyTime_t
    • _PyTime_GetSystemClock()
    • _PyTime_AsSecondsDouble()

  11. scoder commented on Oct 30, 2023

    @scoder
    scoder
    on Oct 30, 2023
    Contributor

    This file uses the following APIs

    Correct. It basically mimics the Python time module and reimplements a part of it in C.

    I'd still recommend exposing the complete C-API. We are talking about a standard library module here, not a part of the core C-API, so it represents a natural API border already. We are not talking about internals here. Let's just keep that up and allow regular external use as before.

  12. vstinner commented on Oct 30, 2023

    @vstinner
    vstinner
    on Oct 30, 2023
    Member

    I designed _PyTime_t API for Python internal usage, but apparently some people discovered it and decided to use it since it's convenient.

    _PyTime_t API was designed to be able to write easily "deadline = time + timeout", but later I started to add "clamp" variants in code which cannot report errors to the caller. Clamp the result in the range [_PyTime_MIN; _PyTime_MAX] is better than having an undefined behavior.

    One issue is that _PyTime_t range is smaller than time_t range:

    // The _PyTime_t API supports a resolution of 1 nanosecond. The _PyTime_t type
// is signed to support negative timestamps. The supported range is around
// [-292.3 years; +292.3 years]. Using the Unix epoch (January 1st, 1970), the
// supported date range is around [1677-09-21; 2262-04-11].

    64-bit time_t (standard even on 32-bit platforms) is way larger than that. In practice, it should not be an issue. But the datetime module cannot use _PyTime_t everywhere for example, since datetime max year is 9999, not year 2262.

    I tried to make the _PyTime_t unit an implementation details and enforce _PyTime_FromNanoseconds() or _PyTime_FromSeconds() usage.

    About raised exception, ValueError is raised for Not-A-Number floating pointer number, and OverflowError on conversions (_PyTime_t, time_t, timeval, timespec, etc.).

  13. encukou commented on Oct 31, 2023

    @encukou
    encukou
    on Oct 31, 2023
    Member

    Victor, do you have any intention of putting the removed API back?

  14. vstinner commented on Nov 1, 2023

    @vstinner
    vstinner
    on Nov 1, 2023
    Member

    Victor, do you have any intention of putting the removed API back?

    My plan in this issue is to add public functions replacing removed _PyTime functions. I'm collecting data to see how the private API was used and which API should be added.

  15. vstinner
    added a commit that references this issue on Nov 16, 2023

    pythongh-110850: Add PyTime_t C API

    c3b4c45

  16. 91 remaining items

  17. vstinner
    added 4 commits that reference this issue on May 4, 2024

    pythongh-110850: Remove _PyTime_TimeUnchecked() function

    6c82b37

    pythongh-110850: Remove _PyTime_TimeUnchecked() function

    68398ec

    pythongh-110850: Remove _PyTime_TimeUnchecked() function

    8888a1e

    gh-110850: Remove _PyTime_TimeUnchecked() function (#118552)

    aa61f8b
  18. SonicField
    added 3 commits that reference this issue on May 8, 2024

    pythongh-110850: Add PyTime_TimeRaw() function (python#118394)

    5cb0055

    pythongh-110850: Use _PyDeadline_Get() in EnterNonRecursiveMutex() (p…

    018b9cc

    pythongh-110850: Remove _PyTime_TimeUnchecked() function (python#118552)

    bc1fc56
  19. LukasWoodtli
    added 8 commits that reference this issue on Jan 22, 2025

    pythongh-110850: PyTime_Time() return 0 on success (pythonGH-115713)

    4141b79

    pythongh-110850: Enhance PyTime C API tests (python#115715)

    2522e95

    pythongh-110850: Replace _PyTime_t with PyTime_t (python#115719)

    01a13f0

    pythongh-110850: Cleanup pycore_time.h includes (python#115724)

    eee3c59

    pythongh-110850: Rename internal PyTime C API functions (python#115734)

    e26a2ad

    pythongh-110850: Use public PyTime functions (python#115746)

    e6dfbb8

    pythongh-110850: Replace private _PyTime_MAX with public PyTime_MAX (p…

    65ede99

    pythongh-110850: Cleanup PyTime API: PyTime_t are nanoseconds (python…

    32b1325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    3.13only security fixesrelease-blockertopic-C-APItype-featureA feature request or enhancement

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions