replacing pkg_resources with importlib

[fondbcn]

Pull Request Checklist

• I’ve read the guidelines for contributing.
• I updated AUTHORS.txt and CHANGES.txt (if the change is non-trivial) and documentation (if applicable).
• I tested my changes.

Description

Sorry for the old error.
I updated the file with this PR instead.

[Kwpolska]

There are some more places where pkg_resources is used, could you fix them all?

[felixfontein]

resources.files() has been added in Python 3.9 (https://docs.python.org/3/library/importlib.resources.html#importlib.resources.files), while this repo still supports Python 3.8 (https://github.com/getnikola/nikola/blob/master/setup.py#L142).

[fondbcn]

resources.files() has been added in Python 3.9 (https://docs.python.org/3/library/importlib.resources.html#importlib.resources.files), while this repo still supports Python 3.8 (https://github.com/getnikola/nikola/blob/master/setup.py#L142).

should we use importlib_resources instead of importlib.resources ?

[Kwpolska]

No. Please use the built-in importlib.resources, but only the functions available in Python 3.7 3.8: https://docs.python.org/3.8/library/importlib.html#module-importlib.resources

[fondbcn]

No. Please use the built-in importlib.resources, but only the functions available in Python 3.7 3.8: https://docs.python.org/3.8/library/importlib.html#module-importlib.resources

I can't, with importlib.resources you can choose only between deprecated or newer methods! (all the available methods in 3.8 are not working with newer python releases)

[felixfontein]

You can always make code depend on sys.version_info.

[fondbcn]

There are some more places where pkg_resources is used, could you fix them all?

not in nikola.py ?

[Kwpolska]

There are some more places where pkg_resources is used, could you fix them all?
not in nikola.py ?

Yes, there are 6 other files.

[fondbcn]

There are some more places where pkg_resources is used, could you fix them all?
not in nikola.py ?

Yes, there are 6 other files.

how to deal with python 3.8 as felixfontein said ?

[felixfontein]

I would probably create a helper function (in utils.py maybe?) which uses the functions available in Python 3.8. The next step would be to make this function depend on the Python version, since Python 3.11 basically deprecated everything in importlib.resources that was available in Python 3.8...

[felixfontein]

Another alternative might be using https://docs.python.org/3/library/pkgutil.html#pkgutil.get_data instead.

[fondbcn]

Another alternative might be using https://docs.python.org/3/library/pkgutil.html#pkgutil.get_data instead.

will "pkgutil" be widely compatible ?

[felixfontein]

According to the docs it's been available since at least Python 2.6.

[fondbcn]

hoping this works.
strangely some files require str() !

[Kwpolska]

1. Instead of repeating the same version check everywhere, please instead make a helper function in utils.py.
2. Please make it so your version check won’t break for Python 4.8, and consider making it <= instead of == while doing that.
3. What do the functions return so that they need str? If it’s a pathlib.Path, it would be desirable to switch other things to Paths instead of working with strings.

[Kwpolska]

The two things are not equivalent, this is why a function in utils would be a better choice.

[Kwpolska]

Please fix the failing tests and linting errors.

[fondbcn]

Please fix the failing tests and linting errors.

I have no idea why resources.path() is not working!

[felixfontein]

Looks like I forgot to send this review. Here it is.

[Kwpolska]

To understand the 3.8 test failures, you can try running the tests locally, but first changing the code so that the 3.8 version runs on all Python versions (or you could install 3.8, but it’s probably not worth it).
The test failures in 3.9+ are unrelated to your code (#3753).

[fondbcn]

To understand the 3.8 test failures, you can try running the tests locally, but first changing the code so that the 3.8 version runs on all Python versions (or you could install 3.8, but it’s probably not worth it). The test failures in 3.9+ are unrelated to your code (#3753).

I tried every method in https://docs.python.org/3.8/library/importlib.html#module-importlib.resources , but nothing is working on 3.8 !
how newer files().joinpath() avoid this issue ???

[felixfontein]

The documentation of importlib.resources.path() says:

Return the path to the resource as an actual file system path. This function returns a context manager for use in a with statement. The context manager provides a pathlib.Path object.

(Emphasis is mine.)

So it returns a context manager, not directly a path. Your code is treating it as a function which returns a path. That doesn't work. Instead of return str(resources.path(package, resource)), you have to do

        with resources.path(package, resource) as path:
            return str(path)

This only works for resources that are not part of an archive. For this to work with resources in an archive, you need to implement a caching solution. Which might be easier if not all uses of resource_filename would be routed through the same utils function call.

I guess one question is how much complexity we want to add for Python 3.8. @Kwpolska what do you think?

[Kwpolska]

This only works for resources that are not part of an archive. For this to work with resources in an archive, you need to implement a caching solution. Which might be easier if not all uses of resource_filename would be routed through the same utils function call.

What’s an archive? We don’t support .egg or zipfile.

If the with statement is enough to fix 3.8, we should do that. If it isn’t, we can use pkg_resources or a backport of importlib_resources on 3.8 (but only 3.8, with a conditional requirement)

[felixfontein]

If we don't support zipfiles or directly running from eggs, then yeah, just doing a return inside the with should be fine.

[fondbcn]

If we don't support zipfiles or directly running from eggs, then yeah, just doing a return inside the with should be fine.

I tried the "with" on path() but the test is still failing.
The documentation of importlib.resources (3.9+) says that path() is equivalent to as_file(files().joinpath()), however read() also didn't work.

[fondbcn]

I found a solution that works for all 3.8 and 3.9+ (at least for me), just adding some init.py in data/ and removing os.path.join .
However the test time is longer.

[Kwpolska]

Please fix the test failures.

[Kwpolska]

Thanks for your contribution!