-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
gh-119164: Make timeit CLI function available from python #119165
base: main
Are you sure you want to change the base?
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Lib/timeit.py
Outdated
raw_timings = t.repeat(repeat, number) | ||
except: | ||
t.print_exc() | ||
run(stmt, setup, timer, repeat, number, time_unit, bool(verbose)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
run(stmt, setup, timer, repeat, number, time_unit, bool(verbose)) | |
_run(t, repeat, number, time_unit, verbose) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add private wrapper
I plan to work on this later today after working on another PR. This will need a news entry but not now. "Weird" is irrelevant. It is common on unix that |
Sorry for the stream of suggestions. I suggest applying them in batch from the files menu: https://github.com/python/cpython/pull/119165/files |
Sorry, I didn't understand I was breaking something with the current code. Missed the documentation part where I have no intention changing the CLI usage at all. Completely agree with you. |
Can you address my feedback now? |
Yes, addressing that now! |
Also, I fixed best being calculated twice, but it seems like that was the case even before my change. |
…d twice, and add internal _run function that accepts a timeit Timer so main can access it.
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Addressed the changes and happy to get another review. The tests should pass but I'm wondering how to change the tests specifically for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Co-authored-by: Nice Zombies <nineteendo19d0@gmail.com>
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
Resolves #119164.
Currently did not verify tests, and have some changes from @terryjreedy to add, so this is still a draft.