jinja2 no longer supports the pytest loader

[gaborbernat]

Currently, jinja2 expects either _path defined or get_filenames: https://github.com/pallets/jinja/blob/master/src/jinja2/loaders.py#L262-L281

The pytest assertion rewriter defines neither (see https://github.com/pytest-dev/pytest/blob/master/src/_pytest/assertion/rewrite.py#L48), and as such running a test suite on a source code that has the following global:

from jinja2 import PackageLoader
LOADER = PackageLoader(__name__, "templates")

will fail with:

    raise ValueError(
E   ValueError: The 'xxx' package was not installed in a way that PackageLoader understands.

Not sure if here jinja2 needs to support more ways to get the template rooot, or pytest loader is missing some methods.

[davidism]

This seems related to #1148. We dropped pkg_resources and are now using pkgutil.get_loader() and loader.get_filename(). If Pytest's loader provides get_filename it should work.

Here's where we detect the package, there are also some workarounds in the surrounding code to support zip and namespace imports:

jinja/src/jinja2/loaders.py

Lines 262 to 267 in 45a76a3

	if hasattr(loader, "get_filename"):
	# A standard directory package, or a zip package.
	self._template_root = os.path.join(
	os.path.dirname(loader.get_filename(package_name)), package_path
	)
	elif hasattr(loader, "_path"):

[gaborbernat]

Pytest's loader does not provides get_filename as I pointed out in the first post. Are you sure that it must though for any loader?

[asottile]

jinja2 is making assumptions about loaders that aren't part of PEP 451 -- it should probably be using the .origin attribute of the module spec in pep451+ worlds

[davidism]

We need a way to get the path to files within a package in order to load them. This appeared to be the way to do that with the APIs provided by Python, but admittedly the documentation about how to do anything with those APIs was pretty much impenetrable for me.

If anyone has a better suggestion that plays nicer with different loaders while still supporting directories, zips, and namespaces, I would appreciate the help.

[davidism]

cc @jaraco

[gaborbernat]

If you want to load some resource shouldn't you be using https://docs.python.org/3/library/importlib.html#module-importlib.resources; there's no need for you to know the source of the file, just what it contains, not?

[davidism]

resources requires all "resource" directories to have __init__.py files, which would be even more incompatible with how Jinja works. See https://gitlab.com/python-devs/importlib_resources/issues/58#note_232533726 for my feedback about that.

[asottile]

this might be a helpful starting point, I'll see if I can shimmy some code into jinja2: https://github.com/asottile/aspy.refactor_imports/blob/519ee18ea75e0045b9b53644c627c6817b2a0748/aspy/refactor_imports/classify.py#L76-L91

[gaborbernat]

Seems this might be an occlusion of the initial design for imporltib.resources; adopting it in its current form though causes regression of features for jinja2; namely that all of sudden no longer supports pytest; so yeah probably @asottile linked code should be added to fill the gap while upstream adds support.

[davidism]

It looks like maybe importlib_resources got an update that allows subdirectories? https://importlib-resources.readthedocs.io/en/latest/changelog%20(links).html#v1-1-0 Might be good to check out as well.

[asottile]

#1169 is an attempt at a fix, I probably still need changelog etc. but you can probably try it out and see if it fixes your issue @gaborbernat

[gaborbernat]

Never mind, the issue is still valid.

[davidism]

Reverted in #1182 back to using pkg_resources for 2.11.2, will use #1169 for 3.0.

[gaborbernat]

Can we have a release with this please?

[davidism]

Waiting on one other PR.

[gaborbernat]

Can you link it? 😄

[asottile]

I suspect it's #1183 given the milestone

[davidism]

Just released 2.11.2 that reverts the change. 3.0 will have the new behavior with the fix.