Skip to content

bpo-36461: timeit - Additional changes for autorange #12954

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

alessandrocucci
Copy link

@alessandrocucci alessandrocucci commented Apr 25, 2019

  • 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.

https://bugs.python.org/issue36461

- have `timeit` and `repeat` methods (and functions) fall back
  on `autorange` if the number is set to 0 or None.
@bsolomon1124
Copy link
Contributor

Was there consideration given to adding autorange() as a module-level function, in addition to being an instance method of Timer as is added here?

Since timeit already uses a "prebound method pattern" to put timeit() and repeat() there, maybe autorange() belongs with them?

@alessandrocucci
Copy link
Author

alessandrocucci commented May 13, 2019

that was not discussed in issue36461
I personally don't think it's necessary, do you have an use case?

@bsolomon1124
Copy link
Contributor

Actually, looking at things more carefully, I don't think it's necessary, since it looks like autorange() is really just meant to be used in timeit()/repeat(), which are already available as module-level functions. So please just disregard my previous comment.

@Carreau
Copy link
Contributor

Carreau commented May 25, 2019

if i understand this correctly max_time_taken is an ill-chosen name, it should be min_time_taken, shouldn't it ?

@alessandrocucci
Copy link
Author

There is a proposal to rename it to "target_time", as Steven wrote in https://bugs.python.org/issue36461

@rhettinger
Copy link
Contributor

ISTM that almost the entire point of having autorange() was to avoid the need for configuration.

Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

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

This has merge conflicts now.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

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

Successfully merging this pull request may close these issues.

9 participants