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.

Sign up for GitHub

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

Jump to bottom

PR: Remove dependency on pkg_resources and use importlib-metadata instead #22244

Merged
merged 1 commit into from
Jul 24, 2024
Merged

PR: Remove dependency on pkg_resources and use importlib-metadata instead #22244

merged 1 commit into from
Jul 24, 2024

Conversation

hmaarrfk
Copy link
Contributor

@hmaarrfk hmaarrfk commented Jul 9, 2024 •
edited by ccordoba12
Loading

It is deprecated and the note says to move away when Python 3.7 is dropped.

Seems like now ;)

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

image
image

Python 3.8
image

Maintenance

Issue(s) Resolved

Fixes #21545

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct:

Mark Harfouche hmaarrfk

@pep8speaks
Copy link

pep8speaks commented Jul 9, 2024 •
edited
Loading

Hello @hmaarrfk! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-07-24 02:12:52 UTC

@hmaarrfk hmaarrfk force-pushed the no_pkg_resources branch 3 times, most recently from ec91cfa to 449e847 Compare July 10, 2024 00:46
@hmaarrfk
Copy link
Contributor Author

i feel like i broke something quite serious and can't get it to work now.

@hmaarrfk hmaarrfk closed this Jul 10, 2024
@hmaarrfk hmaarrfk reopened this Jul 10, 2024
@hmaarrfk
Copy link
Contributor Author

I had a typo so it now runs locally again, but i'm not sure why the CIs are failing so hard.

@hmaarrfk
Copy link
Contributor Author

not sure why this is happening

Error: -10 14:46:39,104 [ERROR] [Installer] -> Missing dependencies  # Mandatory:
  pylint_venv >=3.0.2        :  None (NOK)
  pylsp_black >=2.0.0,<3.0.0 :  None (NOK)

Stack (most recent call last):
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/__boot__.py", line 260, in <module>
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/__boot__.py", line 174, in _run
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/spyder", line 3, in <module>
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/lib/python3.10/spyder/app/start.py", line 270, in main
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/lib/python3.10/spyder/app/mainwindow.py", line 1847, in main
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/lib/python3.10/spyder/app/utils.py", line 333, in create_window
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/lib/python3.10/spyder/plugins/application/container.py", line 526, in report_missing_dependencies
  File "/Users/runner/work/spyder/spyder/dist/Spyder.app/Contents/Resources/lib/python3.10/spyder/utils/installers.py", line 36, in __init__

@ccordoba12 ccordoba12 changed the title Remove dependency on site_packages PR: Remove dependency on pkg_resources Jul 15, 2024
@ccordoba12
Copy link
Member

not sure why this is happening

Do you need a hand with that to finish and merge this PR?

@ccordoba12 ccordoba12 added this to the v5.5.6 milestone Jul 15, 2024
@hmaarrfk
Copy link
Contributor Author

I'm at a loss with the OSX package, a hand would be very helpful.

@hmaarrfk
Copy link
Contributor Author

I rebased on to 5.x into a single commit.

Honestly, I have enough open source street cred, so I'm happy seeing this move forward even if you squash my commits onto yours.

@ccordoba12 ccordoba12 changed the title PR: Remove dependency on pkg_resources PR: Remove dependency on pkg_resources and use importlib-metadata instead Jul 24, 2024
@hmaarrfk @ccordoba12
Remove dependency on pkg_resources
84af373 
It is deprecated and the note says to move away when
Python 3.7 is dropped.

Seems like now ;)
@ccordoba12 ccordoba12 marked this pull request as ready for review July 24, 2024 02:47
ccordoba12
ccordoba12 approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @hmaarrfk!

@ccordoba12 ccordoba12 merged commit b610229 into spyder-ide:5.x Jul 24, 2024
23 checks passed
ccordoba12 added a commit that referenced this pull request Jul 24, 2024
@ccordoba12
Merge from 5.x: PR #22244
da6396b 
Fixes #21545
@hmaarrfk
Copy link
Contributor Author

What did you change????

@ccordoba12
Copy link
Member

I just fixed the importlib.metadata imports in spyder.utils.programs.

@hmaarrfk hmaarrfk deleted the no_pkg_resources branch July 24, 2024 04:57
@hmaarrfk
Copy link
Contributor Author

hmm thanks! sorry i missed that one!

@ccordoba12
Copy link
Member

No prob 👍🏽

I still have to release a new version of python-lsp-server and then we'll release 5.5.6 with your improvements.

mrclary
mrclary reviewed Jul 24, 2024
View reviewed changes
install_dev_repos.py
@@ -15,7 +15,7 @@
from pathlib import Path
from subprocess import check_output

from importlib_metadata import PackageNotFoundError, distribution
from importlib.metadata import PackageNotFoundError, distribution
Copy link
Contributor

@mrclary mrclary Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ccordoba12, do we need a Python version conditional here?

if sys.version_info < (3, 10):
    from importlib_metadata import PackageNotFoundError, distribution
else:
    from importlib.metadata import PackageNotFoundError, distribution

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand very well the difference between importlib_metadata and importlib.metadata, but both PackageNotFoundError and distribution are available in importlib.metadata in Python 3.8 and 3.9.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

importlib_metadata is a backport of importlib.metadata

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To include new features not available in Python 3.8/3.9 importlib.metadata?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes exactly.

But python has been breaking this interface left right and center and not really introducing any meaningful deprecation cycle.

This line of code https://github.com/cupy/cupy/blob/main/cupy/_environment.py#L490

broke my code with importlib_metadata, but i can't recreate in an isolated environment....

@dalthviz dalthviz mentioned this pull request Aug 27, 2024
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ccordoba12 ccordoba12 ccordoba12 approved these changes

@mrclary mrclary mrclary left review comments

Assignees

@hmaarrfk hmaarrfk

Labels
type:Task
Projects
None yet
Milestone
v5.5.6
Development

Successfully merging this pull request may close these issues.
4 participants
@hmaarrfk @pep8speaks @ccordoba12 @mrclary