Skip to content
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

replace time.clock with perf_counter for py3 #638

Closed
wants to merge 1 commit into from
Closed

replace time.clock with perf_counter for py3 #638

wants to merge 1 commit into from

Conversation

frostming
Copy link

Replace the timer to suppress the deprecation warning.

@mfreeborn

This comment has been minimized.

@rsyring
Copy link
Contributor

rsyring commented Mar 9, 2019

#662 uses if sys.version_info >= (3, 3): while this PR uses sys.version_info[:2] >= (3, 3):. I believe both would work given tuple comparison semantics and the former is just a bit cleaner.

Also, we don't currently have CI setup and I don't have easy access to a Windows machine. Will someone manually confirm that this code works as expected on Windows for Python 2.7 and a version of Python3 > 3.3.

@rsyring rsyring added this to the 2.x milestone Mar 9, 2019
@frostming
Copy link
Author

@rsyring I squashed the commits and tested on Windows PC, with Python 3.7.2 and Python 2.7.15, all pass. Thanks

@rsyring
Copy link
Contributor

rsyring commented Mar 11, 2019

Great, thank you. I'll update the milestone to 2.4 as I see no reason to delay this.

@rsyring rsyring modified the milestones: 2.x, 2.4 Mar 11, 2019
@rsyring
Copy link
Contributor

rsyring commented Mar 14, 2019

Would you mind rebasing this on the current 2.x-maintenance branch and change the PR to merge into that branch? I believe that is the last step needed before I can get this merged.

Thanks.

@davidism
Copy link
Member

davidism commented Mar 14, 2019

The command for that would be git rebase --onto origin/2.x-maintenance origin/master deprecate-timer, then git push -f, then edit the PR here to change the target.

@rsyring I usually do this myself, as it's not a common thing for most PR authors to know.

@rsyring
Copy link
Contributor

rsyring commented Mar 14, 2019

@davidism ok, I'll take care of it then. Thanks @frostming for the submission.

rsyring added a commit that referenced this pull request Mar 14, 2019
@rsyring

This comment has been minimized.

@rsyring rsyring closed this Mar 14, 2019
@davidism davidism reopened this Mar 14, 2019
@davidism davidism changed the base branch from master to 2.x-maintenance March 14, 2019 18:56
rsyring added a commit that referenced this pull request Mar 14, 2019
@rsyring
Copy link
Contributor

rsyring commented Mar 14, 2019

@davidism I merged this PR to the maintenance branch, after rebasing again, but the PR didn't pick up that the commit was merged. Let me know here or on Discord if you can tell what I did wrong.

Closing this PR, but it was merged.

@rsyring rsyring closed this Mar 14, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants