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

Upgrade setuptools.depends to importlib from depracated imp #1855

Merged
merged 16 commits into from
Oct 29, 2019
Merged

Upgrade setuptools.depends to importlib from depracated imp #1855

merged 16 commits into from
Oct 29, 2019

Conversation

isidentical
Copy link
Contributor

Summary

Fix;

(.venv) batuhan@x200-trisquel:~/astor$ python setup.py develop
/home/batuhan/.venv/lib/python3.8/site-packages/setuptools/depends.py:2: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details (already added about same subject

@isidentical
Copy link
Contributor Author

apparently the error is related with pytest-dev/pytest#2506

@isidentical
Copy link
Contributor Author

I'll change the get_code api calls to manually compiling / marshalling.

setuptools/tests/test_setuptools.py Outdated Show resolved Hide resolved
setuptools/tests/test_integration.py Outdated Show resolved Hide resolved
@isidentical isidentical closed this Oct 4, 2019
@isidentical isidentical reopened this Oct 4, 2019
@isidentical isidentical requested a review from pganssle October 4, 2019 19:06
@isidentical
Copy link
Contributor Author

All done, it is working as expectedly. No API changed.

@jaraco
Copy link
Member

jaraco commented Oct 27, 2019

I really hate how entangled these behaviors are... and how messy the code will continue to be even after python 2.7 support is dropped. I've got a patch incoming that I think helps.

@jaraco
Copy link
Member

jaraco commented Oct 28, 2019

Almost an hour ago, I thought I'd found a solution, but I realized used of importlib.import_module probably isn't the right usage... that the paths wasn't being honored. So I re-did the work and found that tests were failing, specifically with setuptools._imp.get_module().

I think things are working in that last commit.

@isidentical can you review this latest change and in particular explain what the code in _imp._resolve is meant to do? ...and whether it should be called in _imp.get_module?

Thanks.

@isidentical
Copy link
Contributor Author

now it passes py27 tests (a little fix about imp.load_module

@isidentical
Copy link
Contributor Author

py34 tests are passing (use module_from_spec if py35+)

@isidentical
Copy link
Contributor Author

isidentical commented Oct 28, 2019

@isidentical can you review this latest change

done

explain what the code in _imp._resolve is meant to do?

when i was testing my initial patch i encountered with a case but i dont think it is needed here.

setuptools/_imp.py Outdated Show resolved Hide resolved
@jaraco jaraco removed the request for review from pganssle October 29, 2019 13:32
@jaraco jaraco merged commit 0ced2c0 into pypa:master Oct 29, 2019
@isidentical
Copy link
Contributor Author

Thanks

@jaraco
Copy link
Member

jaraco commented Oct 29, 2019

Thank you. Change is releasing as v41.6.0.

@jaraco
Copy link
Member

jaraco commented Jan 19, 2020

This change was implicated in #1896

@Avasam Avasam mentioned this pull request May 23, 2024
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

Successfully merging this pull request may close these issues.

3 participants