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

Removing pkg_resources #65

Merged
merged 10 commits into from
Dec 14, 2023
Merged

Removing pkg_resources #65

merged 10 commits into from
Dec 14, 2023

Conversation

rnmitchell
Copy link
Contributor

@rnmitchell rnmitchell commented Dec 7, 2023

I keep running into various warnings about package deprecation for pkg_resources. This will remove pkg_resources from lusSTR and replace with importlib_resources.

@rnmitchell
Copy link
Contributor Author

Alright there's something I'm missing here @standage. I've removed the from pkg_resources import resource_filename from all the code (and replaced with a different package) but I'm still encountering a Warning message:

  /Users/rebecca.mitchell/mambaforge/envs/lusstr/lib/python3.7/site-packages/pkg_resources/__init__.py:2871: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('mpl_toolkits')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

What am I missing?

@standage
Copy link
Member

standage commented Dec 8, 2023

I think mpl_toolkits is the key here: is this a MatPlotLib issue? Could upgrading it eliminate the warning message?

@rnmitchell
Copy link
Contributor Author

I'll look into it. It's weird it's popping up with test_snps.py when there's no plotting with SNPs.

@standage
Copy link
Member

I'll look into it. It's weird it's popping up with test_snps.py when there's no plotting with SNPs.

Any luck?

@rnmitchell
Copy link
Contributor Author

It looks like updating to version 3.8.0 should work- although when I try to do this I run into errors with Python versions and macOS versions?

UnsatisfiableError: The following specifications were found
to be incompatible with the existing python installation in your environment:

Specifications:

  - matplotlib=3.8.0 -> python[version='>=3.10,<3.11.0a0|>=3.11,<3.12.0a0|>=3.9,<3.10.0a0|>=3.12,<3.13.0a0']

Your python: python=3.7

If python is on the left-most side of the chain, that's the version you've asked for.
When python appears to the right, that indicates that the thing on the left is somehow
not available for the python version you are constrained to. Note that conda will not
change your python version to a different minor version unless you explicitly specify
that.

The following specifications were found to be incompatible with your system:

  - feature:/osx-64::__osx==12.6.3=0
  - feature:|@/osx-64::__osx==12.6.3=0
  - matplotlib=3.8.0 -> matplotlib-base[version='>=3.8.0,<3.8.1.0a0'] -> __osx[version='>=10.12|>=10.9']

Your installed version is: 12.6.3

@standage
Copy link
Member

If I'm upgrading the Python version in a Conda environment, it's almost always easier to just rebuild the environment from scratch.

@rnmitchell
Copy link
Contributor Author

Upgrading to python 3.10 and Matplotlib 3.8 seems to get rid of this issue. I'm going to close the PR and open a new one.

@rnmitchell rnmitchell closed this Dec 13, 2023
@standage
Copy link
Member

So...pkg_resources is still deprecated, and is unavailable starting in Python 3.12 (I think). So #66 is great, but I think this PR will still be necessary.

@standage standage reopened this Dec 13, 2023
@rnmitchell
Copy link
Contributor Author

This is probably ready for merging, no?

setup.py Outdated
@@ -45,6 +45,7 @@
"snakemake>=7.22.0",
"pyyaml>=6.0",
"matplotlib>=3.5.3",
"importlib_resources",
Copy link
Member

Choose a reason for hiding this comment

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

This is unnecessary. The importlib.resources module is part of the Python standard library starting with version 3.7. The importlib_resources package is only required for compatibility with older versions of Python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to conda install it into my current environment with Python 3.10.

Copy link
Member

@standage standage Dec 13, 2023

Choose a reason for hiding this comment

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

Everywhere you have import importlib_resources you should replace with import importlib.resources, and then the function calls as well. I missed that in my review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh okay! I will fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All fixed.

return resource_filename("lusSTR", "data/filters.json")
return importlib_resources.files("lusSTR") / "data/filters.json"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure it's worth going back at this point, but if you're always using this Path object as a string, you could wrap it with str here instead of every single place you use it in the test suite.

Copy link
Contributor Author

@rnmitchell rnmitchell Dec 13, 2023

Choose a reason for hiding this comment

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

Here or in __init__.py?

Copy link
Member

Choose a reason for hiding this comment

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

Oops, wrong function. I meant to comment on the data_file function in test/__init__.py.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll leave it as is for now. Maybe in the future will change it.

@standage
Copy link
Member

This is probably ready for merging, no?

Clicking the [Ready for review] button to remove the draft status is a great way to indicate this!

@rnmitchell rnmitchell marked this pull request as ready for review December 13, 2023 20:36
@rnmitchell
Copy link
Contributor Author

And of course all this requires the higher Python versions....

@standage standage merged commit 948dd7c into master Dec 14, 2023
2 checks passed
@standage standage deleted the pkg_resources branch December 14, 2023 15:13
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.

2 participants