Deprecating importlib.resources legacy API

Core Development
21

Only after a hard date is determined should you start emitting the always-spams-the-wrong-people stderr by default warning for the requisite policy’s n-release cycles.

I was expecting this error not to be emitted by default but to be flagged in tests.

And in fact, the docs still give me that impression:

DeprecationWarning [is a] Base category for warnings about deprecated features when those warnings are intended for other Python developers (ignored by default, unless triggered by code in __main__ ).

In my experience, a DeprecationWarning is an ignorable warning in the short term, but a call to action (not a call to arms) to address a needed change. By default, a project under pytest will emit these warnings after the test suite run, but won’t block development. I’ll frequently use a revealed DeprecationWarning to identify the project implied in the deprecated behavior to locate or file an issue to correct the behavior and if it’s a slow-moving project, I’ll suppress the warning with a link to the filed issue to avoid being spammed about a known issue. This workflow works really well and it seems to empower projects to make progress in this complex world.

Here I don’t think there’s a one-size-fits-all solution. With the recent deprecation in importlib_metadata, it was the case, pointed out by the SQLAlchemy project (but relevant to others), that the tight constraints of allowed versions were too tight to be reasonably adopted by a library (SQLAlchemy might request >=2.0, but another library might require <2, a true conflict regardless of pip’s handling of it). I acknowledged this concern and provided a specialized workaround to help ease adoption

In the case of this deprecation, importlib_resources 1.3 is the minimum requirement, which has been out for a long time, has Python 2.7 support, and contains no backward-incompatible changes. I’d be shocked if there is a library out there that would encounter a conflict on importlib_resources>=1.3.

I suspect this is a case of over-generalizing the challenges of dealing with backports.

Unfortunately, I don’t have a crystal ball, so I can’t be certain. When it was written and published, it was presumed to be stable and dependable. It was known not to cover all known use cases, but was expected to cover most relating to resources. As it turns out, that assumption was wrong. But even then, it’s not obvious that an incomplete solution couldn’t be extended in a compatible way. It wasn’t until users reported shortcomings and we delved into the problem that we found the existing design to be inadequate for the use-case.

Barry asked me the same question about importlib_metadata, “is it stable, complete?” And my answer was the same - except for the known reported issues in the trackers, it’s stable and complete and I don’t anticipate any more backward-incompatible changes, but I don’t know what important use-cases might emerge that will drive a need to rethink the interface.

I’d like to point out that although the (former) provisional status of importlib.metadata has been raised a few times during this conversation, that status was never used to justify any change.

Is there more that’s not covered? In python/importlib_metadata#38, I did a survey of all of the usage of pkg_resources in setuptools itself and found there were quite a few usages that weren’t met directly. I suspect that the remaining cases can be dealt with in a one-off manner, but I haven’t yet had the time to enact the migration, which may reveal some other important weaknesses that one or both of the libraries should reasonably be expected to address. Still, I’d expect that a backward-compatible solution will be developed unless there’s a strong reason not to do so.

2 Likes
22

In my mind, I’ve gone back and forth on this possibility. And I realize I left out two crucial considerations.

First, leaving the code in place indefinitely leaves a maintenance burden of that code. It remains a surface vulnerable to bugs and security risks and simple user requests. Its mere presence adds a cognitive burden to inspecting the code. And since the code was intended to be deprecated, it’s considered frozen and allowed to be unprotected by tests.

Second, because the backport and CPython implementation are kept in sync, choosing not to remove the functionality from CPython would imply also not removing it from the backport or maintaining divergent implementations.

The goal in deprecating this functionality was to set it on the path to retirement so that what remains after some years is a clean implementation without the burden of legacy code.

Since the steering council has not given any guidance either here or in the relevant pull request, here’s my plan:

  1. Merge the pull request to introduce the deprecation warning on Python 3.11. This will give four Python versions (3.9-3.12) of cross compatibility, such that only 3.8 and 3.13 are incompatible (and thus honoring compatibility across all supported Python versions), a very generous window for functionality that’s available for Python 2.7 (via the backport).
  2. If someone volunteers to carry the maintenance burden of the legacy module (documentation, bug triage, tests, etc) and any divergence in CPython, I’ll allow that person to decide if and how to deprecate the module (including backing out the existing deprecation to a PendingDeprecationWarning or removing the warning completely).
23

To be clear, you didn’t ask the SC for guidance, just Barry and me on that PR. To make a formal request you should open an issue at GitHub - python/steering-council: Communications from the Steering Council or email the SC.

1 Like
24

Good point. Acknowledged. I was imprecise in using SC as shorthand for “members of the SC”. My presumption was that since both Barry and Brett developed the original API, they would have the best familiarity with the implementation but also would have the best instincts as to whether this issue should be brought before the SC.

I’ve identified that importlib.resources in this case is in fact meeting the more conservative standard I paraphrased above:

The backward-incompatible change should not be made until the replacement is available in all supported Python versions, and the DeprecationWarning should not be introduced until as late as possible (two minor releases prior to the removal).

Since this approach meets that conservative standard, the change under consideration isn’t even requesting an exemption from a higher, unpublished standard. I’m uninterested in driving the adoption of a stricter standard for deprecations in the stdlib, which is why I haven’t pushed to get answers to my questions above.

Because this issue has caused a good deal of consternation and hostility, I would like to get feedback from the SC, so I’ve filed a request.

1 Like
25

Currently, test suites of projects that use the API fail (if they treat deprecation warnings as errors, which is a common default).
The error message is: “DeprecationWarning: path is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.” The linked page says to “refer to the _legacy module to see simple wrappers that enable drop-in replacement based on the preferred API, and either copy those or adapt the usage to utilize the files and Traversable interfaces directly.”
However, the wrappers in _legacy use a private _common module (which then uses _compat and _adapters). So simply copying the functions won’t work, and it’s not clear how to adapt them.
And Traversable is currently undocumented.

Since this spans several repos, I’m not sure where to ask for clearer advice.

1 Like
26

Hey Petr.

If a project is treating DeprecationWarnings as errors, they’re adopting the burden to respond to such errors. I maintain a few projects where that’s the case, and I find that approach to be toilsome, requiring emergency response to otherwise mundane changes. Fortunately, the response is usually to (a) capture the failure and suppress the warning or (b) devise a quick fix. IMO, a project should either be responsive to deprecation warnings or tolerant to them, and the recommended approach (as exhibited by the default behavior) is for a project to be tolerant to such warnings.

That feels a little disingenuous, given that you’ve included the hyperlink from the advice directly to the source code (which is pretty straightforward and includes docstrings). Still, I can see why one might desire better documentation.

There is an issue tracking the creation of API docs for the module.

Since the deprecation was introduced and released first with importlib_resources and is not yet released with a public Python, I’d recommend to report it there. BPO would also be fine, but I prefer the Github interface and integrations. Please @mention me on the report, so I can respond promptly.

It’s my understanding that the migration should be fairly straightforward if not intuitive. Where it’s not, I’d like to improve that condition.

27

Hi,
Sorry for sounding antagonistic. I should have made it clear that I want to help, but I couldn’t keep some frustration out of my tone :‍(

The default behavior for test runners is to threat deprecation warnings as errors.
So anyone testing with pre-releases (which we want people to do) has several options:

  1. Ignore the warnings, which will likely make the problem go away for about 2 years. (The bad part here is that library authors who do this pass the problem onto their users, and those have fewer options.)
  2. Fix the issue “the right way”, which is currently not trivial to find if you don’t already know it.
  3. Revert to using pathlib.Path(__file__), which, to some, might seem more stable and straightforward than importlib.resources. If this is seen as an easy way out, that’s a problem for importlib.resources adoption.

I guess the right way is to improve the docs and encourage #2.
Thanks for (re-)pointing me to the Traversable source, that looks like the best source of info. To confirm:

  • Traversable API is meant to be a subset of pathlib.Path, right?
  • importlib.resources.open_text(package, resource) can be replaced by importlib.resources.files(package).joinpath(resource).open(), right?

The problem with source is that it doesn’t usually tell me what details are intended and what’s just an implementation accident (or conversely, a bug).

I asked some specific questions the source doesn’t cover on bpo-47142.

Alas, it is released with a Python version we’re asking people to test…

Since I plan to send a PR and the documentation seems to live in the CPython repo, I opened Issue 47142: Document importlib.resources.abc.Traversable - Python tracker

2 Likes
28

Yes.

That sounds right to me, but as you noticed, the valid values for ‘resource’ are not rigorously defined, especially now that sub-directories are allowed in resources.

Excellent. Thanks. I’ll respond there.

29

Hello,
I believe the removal hurts users (especially since the replacement is not available in still-supported 3.8). I don’t want people who are disillusioned by the complexity to go back to __file__.
And now, I am now in a position where I can commit to supporting the functional API. So, I’d like to do that!
What would be the best way to do so? Should I work in CPython, or send PRs to the external repo?

FWIW, I find the functional API to be much friendlier that Traversable. I’d like to explore keeping it long-term, leaving Traversable targetted more for people who need to subclass it. I believe I can get rid of the drawback of not supporting directories.

7 Likes
30

The issue is at Un-deprecate functional API for importlib resources & add subdirectory support · Issue #116608 · python/cpython · GitHub, with a PR.

This addresses the functions’ main drawback – not allowing subdirectories – by taking multiple path components as positional arguments, for example:

importlib.resources.read_text('modulename', 'subdirectory', 'subsubdir', 'resource.txt')

The additional arguments (encoding and errors) become keyword-only.

There is a wrinkle in this: in Python 3.9-3.11, the above would mean:

importlib.resources.read_text(
    'modulename', 'subdirectory',
    encoding='subsubdir',
    errors='resource.txt',
)

I believe that this is acceptable, since:

  • pragmatically: typical file names do not match typical encoding/errorhandler names
  • lawyerly: the functions have already been deprecated for 2 releases; no one is using them now, riiiight?

However, if this is a problem, I can

  • make the encoding argument required if a text-reading function more than one path component is given.
  • plan to lift this limitation around 3.15.
(click for example) 
importlib.resources.read_text(
    'modulename', 'subdirectory', 'subsubdir', 'resource.txt',
    encoding='utf-8',
)  # OK
importlib.resources.read_text('modulename', 'resource.txt')  # OK
importlib.resources.read_text('modulename', 'subdirectory', 'utf-8')  # error

cc @jaraco

2 Likes
31

:upside_down_face:

Unfortunately in this case, I’m betting most people did I what do and add something like this at the top of their code:

try:
    open_text = importlib.resources.open_text
except AttributeError:
    def open_text(module, name):
        return open(importlib.resources.files(module) / name, "r", encoding="utf-8")

So simply bringing back the name with subtly different behaviour will bring that behaviour in. It’s possible that people check for files() rather than the simpler method, but as I didn’t I can’t suggest assuming or relying on that.

I posted on the issue, but other than that, I’m happy to see this making a comeback. Nothing about its behaviour seemed broken enough to justify straight up deleting the API.

2 Likes
32

On the issue @jaraco said:

Overall, I’m +0 on the change. I’d really like to see more vocal support from other core devs before committing to this approach.

So, please comment there if you have an opinion ­– for or against.

You’re right; the PR now makes encoding required in the ambiguous cases. The limitation can be lifted in a few years.

33

Will the removal of the deprecation warning be backported so that these functions can be freely used on all supported Python versions?

It’s bad enough that new interface uses file system terminology, it would be very unfortunate if people are now using open to read resources from a package. The terminology aside, I find it a little odd that the new Path-like interface is criticised on account of its design when pathlib.Path is roundly preferred to os.path.

1 Like
34

You’re right, I should’ve checked whether there’s a .open(...) I could’ve used, but as this one only affected a test suite it didn’t really matter.

35

There’s quite a few of these in the wild. :smiling_face_with_tear:

36

I don’t think it’s terrible to have the open builtin delegate to a Path object - most builtins already do (e.g. abs(), int(), etc.) - but it probably requires a new protocol to be defined. Might be something @barneygale is interesting in looking at? (In a new place, to avoid derailing this discussion.)

37

Wouldn’t that be changing the semantics of open? I don’t know the ins and outs of the import system, but my understanding is that the return type of files is not an os.PathLike - it’s something that looks like a pathlib.Path but might not actually reside on the file system. open(files(...)) will crash with “files” returned by the zip importer, for instance. I’m not sure that files should be returning pathlib.Paths at all, but yes, that’s probably a discussion for another place. Apologies. (Can a mod split these comments out?)

2 Likes
38

Hence - “requires a new protocol to be defined”. If open begins looking for a new __open_io__ method before converting to a string/path, then the importer handler can provide its own method that returns BytesIO/TextIOWrapper as appropriate. But it can’t be done without changing both sides of that equation - it would, however, avoid the need to change everyone currently using open() on paths that may not be on the filesystem.

1 Like
39

Just chiming in with some points - the discussion here and in the bug is longer than I can wholly digest:

(1) It is okay to change our mind on a deprecation in progress. Or even just question ourselves and delay it longer than originally planned (meaning we haven’t made up our mind or found a reason not to remove it for the time being). So the simple "2. Restore old APIs + functionality as it was." from @barneygale’s issue comment appeals most to me - iff this is to be done at all.

I recommend treading lightly if doing this though.

  • Keep the DeprecationWarning in place for another release before we’re convinced we want to keep them to avoid resetting that clock.
  • Describe in an issue why they aren’t gone yet - what motivated this? - with references from projects using them that do not want to change so we have things to look back at before making our final decision (remove or officially undeprecate) later.

Example reason that if true could be an easy sell to defer behavior changes during 3.13: “existing widely used libraries X, Y, & Z use the APIs and thus removal would slow the ability to test existing important code on 3.13 builds including running free-threading and jit enabled experiments.”

#116608 tries to cover two topics at once. The above is the first one.

(2) “Add subdirectory support” is a feature request and should not be done “at the same time” - not the same PR at the very least. I’d personally say that belongs in its own issue.

Key to adding any new feature is not breaking existing users or adding awkward APIs. The proposed “just accept path parts *args style and join them” caveat of “that’ll conflict with past meanings of additional args, how about requiring an unrelated encoding= keyword argument to trigger a behavior change?” is both breaking and awkward… I think discussions have already covered that.

If the API is kept, this code needs to continue to do the same thing no matter what so long as the API exists rather than change behavior between say 3.10 and 3.15:

importlib.resources.read_text('standard_definitions', 'codecs', 'ascii')

…

Basically I’m calling those both out as wishful thinking false. :slight_smile:

I’m not convinced the old APIs should ever have subdirectory support if retained.

They never had it and we already provide a way to do that.

Rather than “adding subdirectory support” to old not file/io API like read_text, read_binary, open_text, open_binary APIs… Where does the desire come from? “People/just/want/to/type/paths.md” probably? I’m missing a motivating reason for the change.

If there is a motivation, instead consider adding:

importlib.resources.open(package, "look/at/my/slashes", mode="rt")

to naturally mirror the open() API. It adds a single positional first argument and takes a relative path. Disallow non “r”/“rb”/“rt” modes. Forbid absolutes or . and .. traversal. Explicitly do not accept Path-like objects, just a str. All of the remaining arguments should be passed on to / behave exactly as open() does. Probably exclude "open" from __all__ within importlib.resources though to avoid anyone unfortunately doing from importlib.resources import * from becoming secretly surprising…

I expect @jaraco to have convincing arguments about any of this.

Zzz… long message, I guess that was more than a “chime in” :wind_chime:

1 Like
40

Thanks for the points!
Directory support was the main reason to deprecate these functions. That’s my reason to add it “at the same time” as the un-deprecation.

The other thing that’s easy to miss is that I don’t think of these as old functions on life support, but as preferred API for simple use cases. I’d like to target Traversable for people who implement custom finders and want to subclass it, or who want to traverse resource trees. If you want to open/read a file, you don’t need that complexity.

Consider it considered! It’s not a bad idea, but:

  • Resources are immutable (or at least should be treated as such). There are only two modes: text and binary. IMO, separate open_text and open_binary really do work better here.
  • As pathlib experience shows: even if you have open, if all you want is to read the text, read_text is a good shortcut is a good shortcut to the with/open/read chore.
  • It shouldn’t go in the PR that restores old functions :slight_smile:

The slashes are a good idea though; I’ll add them in the next PR update.

Yup, I agree with that now :slight_smile:

With the current version of the PR, this raises TypeError. Granted, during the deprecation period we said the entire function is going away, so people could expect AttributeError or ImportError instead.
Restoring only some calls to their previous behaviour isn’t perfect, but I think it’s fine.

The way to restore previous behaviour is calling with keyword arguments:

importlib.resources.read_text('standard_definitions', encoding='codecs', errors='ascii')

which is not awkward at all. (You probably wanted encoding to be 'ascii', but that’s a point for keyword arguments.)

I wouldn’t want to disallow passing path segments as separate arguments, but I could live with it. But if it is disallowed, I’d still prefer making encoding/errors keyword arguments.