-
Notifications
You must be signed in to change notification settings - Fork 90
Remove fallbacks for unsupported Python versions (< 3.9) #228
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
Conversation
pyperf/_runner.py
Outdated
| if not isolated: | ||
| print("ERROR: CPU affinity not available.", file=sys.stderr) | ||
| print("Use Python 3.3 or newer, or install psutil dependency") | ||
| print("Try a different OS, or install psutil dependency") |
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.
Trying a different OS is a surprising advice. I suggest removing it:
| print("Try a different OS, or install psutil dependency") | |
| print("Install psutil dependency") |
Same remark below (line 424).
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.
Trying a different OS is a surprising advice
I agree. I went for it because on macOS (and others) installing psutil won't help. The feature requires that the OS to expose details of the kernel scheduler. Python's docs only specify "some Unix platforms", psutils specifies it's available on "Linux, Windows, FreeBSD".
How about alternate wording? Maybe: Install psutil dependency and check psutil.Process.cpu_affinity is avaible on your OS.
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.
"Install psutil dependency and check psutil.Process.cpu_affinity is avaible on your OS" sounds better, yes.
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.
Done, and direct link to the pyproc docs in a code comment, for ease of reference.
vstinner
left a comment
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.
LGTM
|
Merged, thanks. |
fixes #227