Skip to content

bpo-40670: Calling timeit.timeit with empty string uses 'pass' by default #20286

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

Closed
wants to merge 3 commits into from

Conversation

DahlitzFlorian
Copy link
Contributor

@DahlitzFlorian DahlitzFlorian commented May 21, 2020

The changes proposed in this PR refer to the discussion over at https://bugs.python.org/issue40670.
The suggestion is to adopt the behaviour of the command-line usage for timeit and use 'pass' if called with '' (empty string).

@remilapeyre what do you think about it?

https://bugs.python.org/issue40670

@edison12a
Copy link
Contributor

@DahlitzFlorian Can you please add news?
Reviewers are more likely to skip a PR if it does not pass all checks.

Copy link
Contributor

@remilapeyre remilapeyre left a comment

Choose a reason for hiding this comment

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

@DahlitzFlorian, I don't think it's a good idea, it will just mask the error and others inputs will make it fail in the same way.

Serhiy Storchaka and Terry Reedy seem to also think that the code should keep behaving that way.

Maybe the documentation could be improved thought?

@edison12a
Copy link
Contributor

When I looked into this; I realized that there are many edge cases that need to be covered for the method to be considered error free.
As per now, First step should be improving the documentation as Terry suggested.
Later, If a comprehensive re-design of the method can be reviewed.

@DahlitzFlorian
Copy link
Contributor Author

The PR was opened pretty early in the discussion. Based on the discussion‘s progress I think, we can close the PR.

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.

5 participants