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

psutil is missing in setup dependencies #70

Open
pzloty opened this issue Jun 13, 2020 · 4 comments
Open

psutil is missing in setup dependencies #70

pzloty opened this issue Jun 13, 2020 · 4 comments

Comments

@pzloty
Copy link

pzloty commented Jun 13, 2020

When running from a virtual environment shell on Windows 10 (2004):
pipenv run python speed_test.py

I got this warning:
WARNING: unable to increase process priority for each process and one extra for each runner.timeit() call.

Figured out that it is because psutil is missing in package dependencies.

@vstinner
Copy link
Member

I got this warning:
WARNING: unable to increase process priority for each process and one extra for each runner.timeit() call.

The minimum work which can be done is to enhance the error message to mention that psutil is recommended on Windows.

Something like:

        if set_cpu_affinity(cpus):
            if self.args.verbose:
                if isolated:
                    text = ("Pin process to isolated CPUs: %s"
                            % format_cpu_list(cpus))
                else:
                    text = ("Pin process to CPUs: %s"
                            % format_cpu_list(cpus))
                print(text)

            if isolated:
                self.args.affinity = format_cpu_list(cpus)
        else:
            if not isolated:
                print("ERROR: CPU affinity not available.", file=sys.stderr)
                print("Use Python 3.3 or newer, or install psutil dependency")
                sys.exit(1)
            elif not self.args.quiet:
                print("WARNING: unable to pin worker processes to "
                      "isolated CPUs, CPU affinity not available")
                print("Use Python 3.3 or newer, or install psutil dependency")

Would you be interested to propose a PR for that?


psutil is mostly useful on Windows. But I didn't feel brave enough to use environment markers in setup.py.

pyperf used environment markers when Python 2.7 was still supported:

diff --git a/setup.py b/setup.py
index fd5207f..3c85bd4 100644
--- a/setup.py
+++ b/setup.py
@@ -59,7 +59,12 @@ def main():
         'author_email': 'victor.stinner@gmail.com',
         'classifiers': CLASSIFIERS,
         'packages': ['perf', 'perf.tests'],
-        'install_requires': ["statistics; python_version < '3.4'", "six"],
+        'install_requires': ["six"],
+        # don't use environment markers in install_requires, but use weird
+        # syntax of extras_require, to support setuptools 18
+        'extras_require': {
+            ":python_version < '3.4'": ["statistics"],
+        },
         'entry_points': {
             'console_scripts': ['pyperf=perf.__main__:main']
         }

which caused issues with old setuptools versions: https://pyperf.readthedocs.io/en/latest/run_benchmark.html#install-pyperf

I'm open to reintroduce environment markers to require psutil, but only on Windows.

Another option is to write a C extension just for set_cpu_affinity().

Another option is to simply make the warning quiet on Windows.

@pzloty
Copy link
Author

pzloty commented Jun 18, 2020

Yes, adding another line to the warning would be sufficient with minimal code.:

def _process_priority(self):
    if not MS_WINDOWS:
        return
    if not set_highest_priority():
        print("WARNING: unable to increase process priority")
        print("Use Python 3.3 or newer, or install psutil dependency")  # <---

My first choice was:

install_requires=[
        "psutil>=5.7;platform_system=='Windows'"
    ]

But I understand that it's not required for basic functionality.

I got the warning because of the set_highest_priority().

Suppressing warning/error/exception leads to confusion.
It would be much nicer to display a warning as soon as psutil import fails. E.g. by wrapping it:

Replacing all import psutil with:

import pyperf._psutil as psutil

And then in _psutil.py file:

try:
    # Optional dependency
    from psutil import *
except ImportError:
    __IMPORT_FAILED = True
    raise ImportError("psutil")
else:
    __IMPORT_FAILED = False
finally:
    if __IMPORT_FAILED:
        print("Optional dependency 'psutil' is not available: some features will not be available.")

And then in set_highest_priority() and set_cpu_affinity() check psutil against None instead of trying to reimport it. Is there any reason for that reimport?
But I'm not sure if it has any negative consequences.

@vstinner
Copy link
Member

install_requires=["psutil>=5.7;platform_system=='Windows'"]

If you propose a PR to implement that, I can test it ;-)

@gvanrossum
Copy link

I get this too with 3.11 built from main, today. It seems psutil is not compatible with the latest main branch. Though it looks like psutil still builds with 3.11a6, and then the set_highest_priority() function (in _cpu_utils.py) returns True, so you wouldn't get the error.

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

No branches or pull requests

3 participants