Skip to content

gh-80642: timeit - make target time of autorange configurable#140283

Merged
hugovk merged 20 commits intopython:mainfrom
miikka:fix-issue-36461
Apr 3, 2026
Merged

gh-80642: timeit - make target time of autorange configurable#140283
hugovk merged 20 commits intopython:mainfrom
miikka:fix-issue-36461

Conversation

@miikka
Copy link
Copy Markdown
Contributor

@miikka miikka commented Oct 18, 2025

This is an updated version of PR #12954 with latest main merged in. I also fixed a few issues in the original (it's now consistently called target_time, there was a float conversion missing, etc.).

To quote the original PR:

  • make the 0.2s time configurable;
  • have timeit and repeat methods (and functions) fall back on autorange if the number is set to 0 or None.

📚 Documentation preview 📚: https://cpython-previews--140283.org.readthedocs.build/

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot bot commented Oct 18, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

miikka and others added 2 commits October 18, 2025 13:22
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
@picnixz
Copy link
Copy Markdown
Member

picnixz commented Oct 19, 2025

Previously timeit returned a single value. Now it can return a pair if number is 0. This should be better documented because it coule be breaking existing code (I do not know if 0 was a legit value beforehand). I do not think we should expose the handling of number=0 to the timeit method itself. Or if this is the case we should first determine the number by autorange and then pass it to timeit() for the "real" test.

I cannot comment on the PR because the mobile app is not working well so I am only commenting here, but please consider this as me requesting changes.

Co-authored-by: Stan Ulbrych <89152624+StanFromIreland@users.noreply.github.com>
@miikka
Copy link
Copy Markdown
Contributor Author

miikka commented Oct 22, 2025

Previously timeit returned a single value. Now it can return a pair if number is 0. This should be better documented because it coule be breaking existing code (I do not know if 0 was a legit value beforehand). I do not think we should expose the handling of number=0 to the timeit method itself. Or if this is the case we should first determine the number by autorange and then pass it to timeit() for the "real" test.

When I resurrected this PR, I thought this was a plausibly-useful convenience feature. However, I'm not so sure anymore:

  • 0 was a legit value beforehand in the sense that it'd run the timing logic but it would not execute stmt at all. There's an unit test for the case, too. Is this useful? I browsed through a GitHub code search and it seems there are people using number=0 to essentially disable the timeit call. Returning a pair would break this usage.
  • You have to return the pair, though, because you can't really use the result for anything without knowing both how long it took and the iteration count.

I'm inclined to undo the change. An alternative option would be to provide a new top level convenience method timeit.autorange() that would create a Timer instance and call autorange, like the other convenience methods work. I'm not sure if this would be appropriate.

@miikka
Copy link
Copy Markdown
Contributor Author

miikka commented Oct 22, 2025

Looking back at @stevendaprano's original idea, the proposal was that the autorange fallback would be used when number is 0 or None. A non-breaking alternative would be to use autorange when number is None. None has not been a legit value before (using it raises a TypeError).

@hugovk
Copy link
Copy Markdown
Member

hugovk commented Nov 5, 2025

@picnixz Do you have any preferences here?

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Nov 6, 2025

FTR, I've seen your ping Hugo and will reply to this tomorrow or on Saturday (just need some time)

@hugovk
Copy link
Copy Markdown
Member

hugovk commented Feb 20, 2026

@picnixz reminder :)

@picnixz
Copy link
Copy Markdown
Member

picnixz commented Feb 20, 2026

Sorry Hugo, I saw the ping, then forgot about it. You can usually ping me earlier in general as well :) Now:

  • Whatever choice we do, we can't change the return type of a public method. And we should not mix return types depending on the arguments either.
  • I still don't understand why we should alter the timeit method itself. What we want is removing the hardcoded 0.2 in autorange(). Adding a parameter to autorange() is fine. The change in timeit() does not seem related (or am I just missing something?)

So I'm inclined to accept the change for configuring the hardcoded 0.2 (it's not an issue there) but I want another discussion about using None or 0 in timeit() and other methods to have "empty" loop.

@hugovk
Copy link
Copy Markdown
Member

hugovk commented Feb 20, 2026

So I'm inclined to accept the change for configuring the hardcoded 0.2 (it's not an issue there) but I want another discussion about using None or 0 in timeit() and other methods to have "empty" loop.

Yes, sounds reasonable.

Once this PR is done, I think we could prompt for further discussion in the issue, and even put pending on it and close it if there's not much further interest for the extra stuff.

@miikka Please can you update this PR?

Thanks both!

@miikka
Copy link
Copy Markdown
Contributor Author

miikka commented Apr 1, 2026

Hi, sorry for taking a while to get back to this. To recap: the main goal of the PR is make the hardcoded 0.2 seconds configurable in autorange(). I've undone the changes to the timeit(). However, I did keep the non-breaking change to add --target-time parameter to the python -m timeit CLI tool.

Copy link
Copy Markdown
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Could you also add a What's New entry for this new feature please?

Lib/timeit.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[-t T] is missing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed

@miikka miikka requested a review from AA-Turner as a code owner April 2, 2026 05:52
@miikka
Copy link
Copy Markdown
Contributor Author

miikka commented Apr 2, 2026

Thanks. Could you also add a What's New entry for this new feature please?

Added a What's New entry

Copy link
Copy Markdown
Member

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Sprint 2024 Apr 2, 2026
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@hugovk hugovk enabled auto-merge (squash) April 3, 2026 06:31
@hugovk hugovk merged commit f3b74d6 into python:main Apr 3, 2026
51 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Sprint 2024 Apr 3, 2026
@hugovk
Copy link
Copy Markdown
Member

hugovk commented Apr 3, 2026

Thank you!

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

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants