Repository navigation
Please expose _PyTime_t, _PyTime_FromTimeval, and _PyTime_AsSecondsDouble as public APIs #110850
Description
Activity
- addedtype-featureA feature request or enhancementA feature request or enhancement
on Oct 14, 2023 - 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 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
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.
Reacted by Hugo van KemenadeI'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.
We actually expose that API in Cython's
cpython/time.pxd. Please move it back to where it was.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()
This file uses the following APIs
Correct. It basically mimics the Python
timemodule 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.
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_trange 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_teverywhere for example, since datetime max year is 9999, not year 2262.I tried to make the
_PyTime_tunit 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.).
Victor, do you have any intention of putting the removed API back?
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.
- added a commit that references this issue
on Nov 16, 2023 91 remaining items
- added 4 commits that reference this issue
on May 4, 2024 - added 8 commits that reference this issue
on Jan 22, 2025
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
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