Deprecating importlib.resources legacy API

Thanks for the discussion, everyone!

The PR is ready for merging. It is already merged to the importlib_resources backport.

To recap:
The resource name may contain slashes; and multiple components can be given as multiple arguments. This matches Traversable.joinpath, and pathlib.Path.joinpath (on Posix). Matching Traversable.joinpath means that I’m not making any (new) design decisions here, like whether to allow ../.
For backwards compatibility, if you pass multiple arguments to text functions, you need to add encoding argument. This weird wart is only in the new feature (multiple arguments), and it can be removed 3.15. Until then, if you don’t like it, you can use '/'.join :‍)

Old calls will work except the encoding and errors arguments are now keyword-only; using them positionally results in TypeError.

Since this is adding the feature that caused the deprecation, I feel justified un-deprecating in the same PR.

I’m sorry, I don’t think I agree that changing the API in a backward-incompatible way is warranted just because the deprecation is cancelled. You can deal with deprecations with feature tests, but you can’t do that for these backward-incompatible changes. I think the only sensible way here is to keep old code working as-is, which means passing path elements as positional arguments should require passing encoding/errors as keywords, instead of making encoding/errors keyword-only.

I think that turning some old code into errors, but keeping all other old code working as-is, is sensible too. But, if you find it inacceptable, it can be changed.

With the PR, they do require that (if there’s more than one positional argument).

Making them keyword-only makes ambiguous calls like the following raise TypeError:

read_text('foo', 'bar', 'baz')

The user must specify what they mean, one of:

  • read_text('foo', encoding='bar', errors='baz')
    
    (which is backwards-compatible; no need to also add feature tests)
  • read_text('foo', 'bar', 'baz', encoding='utf-8') 
    
    (which fails on old versions; on new ones it’s the same as read_text('foo/bar/baz'))

Again, if you think this call should have the old behaviour, rather than raise an error, it’s an easy enough change.

Apologies for insisting and I don’t know if other people feel the same way, but I’m not sure if I understand the benefit of backtracking on the old API now, that the community’s already gone into the effort of migrating to the new, “fluent” API, either completely or by creating backward-compatible wrappers, which they will also have to maintain indefinitely to silence the deprecation warning in older Python versions. Is importlib.resources.read_text(module, 'a', 'b') significantly better than importlib.resources.files(module).joinpath('a', 'b').read_text()? Is it worth having two very similar APIs which accomplish the same thing? Would it be helpful to have to explain to users that they can’t use the old API in older Python versions (or at all) if they’re recursing into a package? And can the deprecation period simply not be extended until the oldest supported Python version has the new API, if you feel that the removal is premature?

The community is big; not all of it has migrated. I agree it would have been nice to do this sooner.

As for whether it’s worth it, I’m too biased to judge that. I get a disproportionate amount of grumbling from people who patch older libraries to keep them working.
But see the issue – and this thread – for several +1s.

3 Likes

Perhaps, but a big portion of the FOSS community has migrated. I think whatever churn has for a large part already been incurred should be weighed against the cost of leaving this module in a permanent internally-inconsistent state with conflicting (resource vs file, “open_bytes” vs “open_binary”) and sometimes unclear (“path”) terminology, subtly different behaviour between the functional and fluent APIs, and the nitpick-ity arguments that the choice of the two APIs will inflict.

With a comment from a Numpy/SciPy maintainer in support of the PR, I’ll merge it.

Thanks everyone for sharing your concerns, and sorry that I can’t make everyone happy.

There’s still time to revert the multiple-args, or the whole un-deprecation­ – convince me, or bring it to the SC.


My opinions (weakly held):

Is it worth having two very similar APIs which accomplish the same thing?

I do think it’s good to have this kind of façade on top of the Traversable API, like we have import on top of finders and loaders, or len() to call .__len__(). One for everyday use, one to implement the internals or for special uses.

Would it be helpful to have to explain to users that they can’t use the old API in older Python versions (or at all) if they’re recursing into a package?

That’s quite normal for a new feature.
But if we’re talking about old Python versions, note that the still-supported Python 3.8, doesn’t have importlib.resources.files. I assume that’s why some projects are delaying to switch (and welcoming the un-deprecation).
I assume you knew that given the next question:

can the deprecation period simply not be extended until the oldest supported Python version has the new API, if you feel that the removal is premature?

That’s definitely a possibility! But why not make the deprecation period even longer than that? After all, some portion of the community still targets LTS Linux distros, with longer support than you get for python.org builds.
(Funnily enough, a question like this started me on the way to write this PR.)

5 Likes

I don’t think this is much of a “facade” in the way that you are describing (I don’t see anyone suggesting we need a facade on top of pathlib.Path, for instance, which the Traversable interface imitates), but it’s not such a big deal at the end of the day, so I’ll stop beating this horse. :wink: Thanks for working on restoring/modernising the old API.

3 Likes

I realize this discussion is stale, just wanted to thank you for doing the IMHO sensible thing.

I am late to the party and just noticed that my client code to read a resource was supposed to be changed from

     component_schema = resources.read_text(schema_package, "component.schema.json")

was supposed to read

    component_schema = resources.files(schema_package).joinpath("component.schema.json").read_text()

I deem the original code far more readable, but got deprecation warnings. Turns out, this has been un-deprecated again :slight_smile:

Thanks!

1 Like