-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Get rid of distutils
#11300
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
Get rid of distutils
#11300
Conversation
from distutils.sysconfig import get_python_lib | ||
from sysconfig import get_python_lib |
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.
I think this is only run on python 2 since getusersitepackages
and getsitepackages
were added in python 3.2, so it's probably fine to leave this using distutils.
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.
disutils
is becoming a blocker for #11297. It makes sense to me to guard the python version here.
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.
I don't think guarding the python version is necessary because this code will never get run on python 3 anyways (although it probably wouldn't hurt either).
None of the DeprecationWarning
s that I see in the CI run for #11297 seem to be because of this usage. All of those warnings seem to be either from mypyc/build.py or from the setup.py that mypyc generates.
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.
oh I thought it was causing error all over the codebase, my miss
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.
@pranavrajpal is correct. I already fixed this some time ago by moving the import from the top-level to the else-clause: #10203
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.
Yeah, agreed with pranavrajpal on not needing to change pyinfo.py. I think we still need the distutils API, but we'll want to get it from setuptools if possible. I opened #11306 for this.
Thanks for pointing out! I will fix rest erros directly in this PR #11297 |
Description
Replaces
distutils
with other modules.Test Plan
Shouldn't affect any tests.