time: Make _ClockInfo concrete and simplify platform checks
#13413
Conversation
| def time() -> float: ... | ||
| def time_ns() -> int: ... | ||
| def monotonic() -> float: ... | ||
| def monotonic_ns() -> int: ... | ||
| def perf_counter() -> float: ... | ||
| def perf_counter_ns() -> int: ... | ||
| def process_time() -> float: ... | ||
| def process_time_ns() -> int: ... | ||
| def thread_time() -> float: ... | ||
| def thread_time_ns() -> int: ... |
There was a problem hiding this comment.
Also put the seconds and ns clock functions together
hamdanal
changed the title
_ClockInfo concrete andsimplify platform checks_ClockInfo concrete and simplify platform checks
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
| class _ClockInfo(Protocol): | ||
| class _ClockInfo(SimpleNamespace): |
There was a problem hiding this comment.
Hmm, this change isn't entirely accurate though, right? At runtime it actually is an instance of SimpleNamespace, not an instance of a subclass of SimpleNamespace:
>>> import time
>>> i = time.get_clock_info("monotonic")
>>> i
namespace(implementation='mach_absolute_time()', monotonic=True, adjustable=False, resolution=4.166666666666666e-08)
>>> type(i)
<class 'types.SimpleNamespace'>so the fully accurate type hint for get_clock_info would be
def get_clock_info(name: Literal["monotonic", "perf_counter", "process_time", "time", "thread_time"], /) -> SimpleNamespace: ...but that would suck for users, because any attribute access on a SimpleNamespace attribute is inferred as Any by type checkers.
Could you say a little bit about what problem this change solves? I think type checkers should be able to infer that the object returned by get_clock_info cannot have its __class__ set to _ClockInfo exactly -- since _ClockInfo is an abstract class, it must be an instance of a concrete class that satisfies the _ClockInfo interface that is returned, rather than an instance of _ClockInfo exactly
There was a problem hiding this comment.
TBH, this change has been sitting in my notes for a very long time I forgot what I was doing with this function at the time. Please feel free to close the PR if you think the change is not correct.
There was a problem hiding this comment.
Alright, no worries. I'm happy to consider a change along these lines if there's a concrete problem it's solving, but to me it looks like it makes the signature slightly less accurate, so I'm inclined to close it if not :-)
No description provided.