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

replaced pkg_resources with importlib.resources #2232

Conversation

michaelraczycki
Copy link
Contributor

@michaelraczycki michaelraczycki commented Apr 10, 2023

Description

Replacement of deprecated pkg_resources, now arviz will use importlib.resources
which will allow to use more strict testing policy in packages importing arviz ( previously because of pkg_resources
failing test on warning was not possible, because each import of arviz was warning about deprecation of pkg_resources)


📚 Documentation preview 📚: https://arviz--2232.org.readthedocs.build/en/2232/

Copy link
Member

@OriolAbril OriolAbril left a comment

Choose a reason for hiding this comment

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

It looks like the part about the static files is not working after these updates and ArviZ can't be imported, failing all tests. Are you able to import locally after your changes?

Copy link
Contributor

@ahartikainen ahartikainen left a comment

Choose a reason for hiding this comment

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

I think python3 actually reads bytes when calling pkg_resources resource_string

Edit. Oh read_text is fine, but maybe we need to change how we read it

arviz/utils.py Outdated
@@ -658,7 +658,7 @@ def _load_static_files():

Clone from xarray.core.formatted_html_template.
"""
return [pkg_resources.resource_string("arviz", fname).decode("utf8") for fname in STATIC_FILES]
return [importlib.resources.read_text("arviz", fname) for fname in STATIC_FILES]
Copy link
Contributor

@ahartikainen ahartikainen Apr 11, 2023

Choose a reason for hiding this comment

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

Suggested change
return [importlib.resources.read_text("arviz", fname) for fname in STATIC_FILES]
return [importlib.resources.files("arviz").joinpath(fname).read_text() for fname in STATIC_FILES]

edit. read_text is fine just change how it reads it.

@ahartikainen
Copy link
Contributor

Ok, it fails because files API came in 3.9.

@OriolAbril when are we going to ditch 3.8 support?

@OriolAbril
Copy link
Member

We could do it now : https://scientific-python.org/specs/spec-0000/. It'd be best to check dependencies with the spec on a different PR then rebase. Any takers?

@michaelraczycki
Copy link
Contributor Author

I could do it (probably closer to the weekend as now I want to finish the ModelBuilder PR on pymc-experimental)
So what's needed is to create another branch, then change the python version to 3.9 in setup.py and push it to see if the CI can correctly build and install it?

@OriolAbril
Copy link
Member

OriolAbril commented Apr 11, 2023

Already in progress at #2233 @michaelraczycki. It looks like we'll have to wait a bit though.

@michaelraczycki
Copy link
Contributor Author

Already in progress at #2232 @michaelraczycki. It looks like we'll have to wait a bit though.

I think you've linked this issue :)

@OriolAbril
Copy link
Member

Fixed, sorry

@OriolAbril
Copy link
Member

OriolAbril commented May 8, 2023

I closed the PR accidentally, reopening again seems to create a new one. @michaelraczycki can you re-open it? (either here or from main...michaelraczycki:arviz:pkg_resources_replacement) it should already be rebased and passing all checks

@OriolAbril
Copy link
Member

Ping @michaelraczycki

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