GeezerVison is showing github.com — Better docs for importlib.resources · Issue #93610 · python/cpython
Better docs for importlib.resources #93610DescriptionThe current documentation of importlib.resources can be improved.
The main hindrance to usability are references to classes that are not linked correctly, leaving the docs very hard to follow for someone who doesn't already know the module. For example, Traversable in the docs of files is not a link. Activityencukou commented on Jun 8, 2022 warsaw commented on Jun 10, 2022 Just ran across this one too! importlib.resources.rst links to importlib.resources.abc.Traversable but it should be importlib.abc.Traversable. PR coming for that.
@encukou were there other problems you found? added 4 commits that reference this issue on Jun 10, 2022This issue will close once commit 516f14d is merged into the 'main' branch.516f14d This issue will close once commit a43d8dc is merged into the 'main' branch.a43d8dc pythongh-89896: `importlib.abc.Traversable.name` is an attribute, not… This issue will close once commit 1f47e59 is merged into the 'main' branch.1f47e59 warsaw commented on Jun 11, 2022 I've opened two PRs which seem to fix the documentation problem, but I'm not actually sure this is the right thing to do. The deal is that in 3.11, Traversable, ResourceReader, and TraversableResources were moved to importlib.resources.abc, with this compatibility hack added to importlib.abc:
# for compatibility with Python 3.10
from .resources.abc import ResourceReader, Traversable, TraversableResources
However, the documentation for importlib.resources.abc is included in importlib.rst (inside the :mod:importlib.abc section):
.. include:: importlib.resources.abc.rst
which is why (I think), the doc cross references to importlib.resources.abc.Traversable don't work (without the above PRs). I don't see any mention of this reorg in the What's New in Python 3.11 docs, nor in the importlib.abc documentation, so I'm not sure what the intended public API for these ABC's is.
@jaraco can you weigh in? encukou commented on Jun 13, 2022 Yes, that confused me as well, so I went with the implementation here – <class 'importlib.resources.abc.Traversable'>. encukou commented on Jun 13, 2022
were there other problems you found?
I sent a PR with improvements, it's probably better to look there, but:
- for migrating away from the deprecated functions, there's a link to importlib_resources docs, which says to look in the source of
importlib_resources._legacy – which uses a private module for all of the implementation. (Perhaps with Traversable documentation being discoverable, this would be easier to use?)
- the deprecated functions take up most of the page, and could be moved to a new “you don't need to read this” section
- many references to types/functions could be links to the relevant docs, so they're more usable for people who don't already know them
- it's pure reference; there's no hint about why you would use this and not just
__file__
jaraco commented on Jun 13, 2022
I don't see any mention of this reorg in the What's New in Python 3.11 docs, nor in the importlib.abc documentation, so I'm not sure what the intended public API for these ABC's is.
At the time of writing, I considered the ABCs to be primarily an implementation detail, maybe occasionally referenced in the source for clarity. I wasn't even sure there should be a concrete ABC to describe the interface, but as it developed, the value of the abcs became more apparent and started having references in the documentation.
My preference is for code to be self-documenting as much as possible, limiting the amount of duplication that has to happen in prose.
When I moved the module, I still considered the Traversable object to be primarily an implementation detail, although by moving it to the abcs module, I was preparing it to be supported first-class for type hints.
It looks like the Traversable class was moved in python/importlib_resources#87, quite some time ago. Oh, that's not when the content moved to its own module under resources. bpo-46118 is where that work happened. Perhaps this news entry could have been more explicit about the move of some interfaces from importlib.abc to importlib.resources.abc. encukou commented on Jun 13, 2022 So, should it be documented as importlib.resources.abc.Traversable or importlib.abc.Traversable? warsaw commented on Jun 13, 2022
Yes, that confused me as well, so I went with the implementation here – <class 'importlib.resources.abc.Traversable'>.
Except that link doesn't work.
So, should it be documented as importlib.resources.abc.Traversable or importlib.abc.Traversable?
If the latter, my PR fixes that link. If the former, perhaps it shouldn't be :include:'d in importlib.abc docs? encukou commented on Jun 14, 2022 If the former, my PR fixes the link, by moving importlib.resources.abc to its own page. encukou commented on Jun 15, 2022
importlib.resources was added to Python in 3.7
Everything added in 3.7 is now deprecated. The current API is from 3.9. warsaw commented on Jun 15, 2022
Everything added in 3.7 is now deprecated. The current API is from 3.9.
I guess that's a valid reason for keeping the standalone package around, but at some point (maybe 3.9 EOL?) it's likely not worth it. Then again, that's a @jaraco decision. jaraco commented on Jun 16, 2022
Everything added in 3.7 is now deprecated. The current API is from 3.9.
I guess that's a valid reason for keeping the standalone package around, but at some point (maybe 3.9 EOL?) it's likely not worth it. Then again, that's a @jaraco decision.
I'd not say the current API is from 3.9. A major change landed in 3.9, but more changes landed in 3.10 and some have yet to land (for example, python/importlib_resources#250 was inspired by recent feedback). You can see in the compatibility matrix which versions of _resources map to which versions of CPython, and you can use the history to see what changes occurred between those versions. So users who want features from importlib_resources 5.1 or later will need to use Python 3.11 or importlib_resources.
If at some point, you want not to support backporting any behavior, we should make that decision, but the status quo is to develop the changes in the backport, prove their viability, then port them into the next CPython. Same is true for importlib.metadata.
I'd really like for @jaraco to weigh in. I'm guessing he'd prefer to split it into importlib.resources.abc since it will be easier to keep the standalone backport in sync. OTOH, importlib.resources was added to Python in 3.7, so how much longer does importlib-resources still need to be actively maintained? It had an upload back in April, but I don't know what the roadmap is for that package.
There's no roadmap per se. It's pretty stable, but if there's an improvement added in the latest version, it improves adoption to be able to make that feature available in the backport.
Yes, I'd prefer to keep abcs for resources in that package. Even in isolation, it's better to have abcs in the same package where they're used for logical encapsulation (for the same reason that you don't put abcs from collections.abc and importlib.abc in a top-level .abc).
That said, it also helps the backport immensely, since the code that exists in 'importlib_resources' has a direct mapping to the same module in importlib.resources.
If it was up to me (and maybe it's good that it's not!), I'd schedule importlib-resources for retirement and stop development on it. Then I would probably leave Traversable and such in importlib.abc, without the artificial distinction. But that's just me.
I would like to retire the backport too, but I also find it 10x easier to develop in the backport (massively better test runner, faster tests, no compiler needed, fast iteration, coverage testing, ...). Let's not make that decision an an "improve docs" bug. Maybe file a bug with importlib-resources?
For all the reasons above, let's use importlib.resources.abc. It's a better choice. I'll update the what's changed to include that move. Let's get the docs to match. encukou commented on Jun 16, 2022
If at some point, you want not to support backporting any behavior, we should make that decision, but the status quo is to develop the changes in the backport, prove their viability, then port them into the next CPython. Same is true for importlib.metadata.
IMO, that's a good strategy. The downside is that since the docs aren't synced with the backport, they feel somewhat second-class – they're split between CPython docs (reference) and backport docs (guides), and references to the docs (or to source code) seem awkward.
I hope that doesn't read as too negative. I mean it as constructive criticism – pointing out what I see as weak spots so I can help make them better.
And I don't mean to imply you need to work on the docs – if it's a chore you'd rather delegate, just say so! IMO there are people who'd love to work on this but are somewhat wary of touching “someone else's” docs.
For all the reasons above, let's use importlib.resources.abc. It's a better choice. I'll update the what's changed to include that move. Let's get the docs to match.
@warsaw, do you want to review #93611?
@jaraco, what's the plan for having Traversable importable from importlib.abc? Is that temporary, or should it be documented and supported? jaraco commented on Jun 16, 2022
they feel somewhat second-class – they're split between CPython docs (reference) and backport docs (guides)
And this split is inconsistent across .metadata and .resources, because docs for .resources were developed primarily in CPython but docs for .metadata were developed primarily in the backport. I'm unsure which model is preferable, but I'd like to align them. I lean slightly toward the .resources model, where most of the docs (reference and guides) are in CPython and maintained there, with backport-specific details in the backport.
See python/importlib_resources#240 where I've invited others to contribute to the docs.
wary of touching “someone else's” docs
Ultimately, I want not to be a blocker or hindrance to contribution, but I do want to avoid increasing maintenance toil. I'd prefer to have a development partner who can share ownership of the design, and reserve drive-by contributions to those that are mostly uncontroversial.
@jaraco, what's the plan for having Traversable importable from importlib.abc? Is that temporary, or should it be documented and supported?
I'd say the presence in importlib.abc is deprecated. I'm unsure if it actually needs to go through a deprecation process, since it wasn't documented. encukou commented on Jun 16, 2022
Ultimately, I want not to be a blocker or hindrance to contribution, but I do want to avoid increasing maintenance toil. I'd prefer to have a development partner who can share ownership of the design, and reserve drive-by contributions to those that are mostly uncontroversial.
What do you mean by sharing ownership of the design?
I'd say the presence in importlib.abc is deprecated. I'm unsure if it actually needs to go through a deprecation process, since it wasn't documented.
It turns out it is documented in 3.10.
It's importlib.resources.abc.Traversable that isn't documented (and that's the name mentioned in the function docs). jaraco commented on Jun 16, 2022
What do you mean by sharing ownership of the design?
I just mean I'm looking for someone who cares about the product and the design largely, someone invested enough to understand the motivations behind the existing implementation.
It turns out it is documented in 3.10.
It's importlib.resources.abc.Traversable that isn't documented (and that's the name mentioned in the function docs).
Okay. Then a deprecation process is probably in order. Let's get the docs to point to the preferred location (resources.abc) and worry about the deprecation separately. jaraco commented on Nov 26, 2022 I appreciate all of work you've done, @encukou . Is there anything more you'd like to see here? I'm closing this as complete, but feel free to re-open or otherwise revive the conversation. aiven-anton commented on Dec 6, 2022 Hi, I was about to open a new issue but noticed this discussion seems somewhat recent.
Currently importlib.metadata.distributions (link) is not documented. Is this covered by the PRs that are already merged?
It seems like this full API reference from the importlib-metadata project should somehow make its way into the CPython docs?
For this specific case, is it safe to consider distributions public?
Please let me know if I should open a separate issue. jaraco commented on Dec 6, 2022
Currently importlib.metadata.distributions (link) is not documented.
This issue is mainly about importlib.resources, which is different from importlib.metadata, though I too get those mixed up sometimes, and this issue did touch on importlib.metadata.
It seems like this full API reference from the importlib-metadata project should somehow make its way into the CPython docs?
Unfortunately, last I checked, there's no support in CPython for generating the API docs from the source (where the behavior is documented). See python/importlib_metadata#119 for some work in this area. The solution at the time was for the CPython docs to link back to the API docs in importlib_metadata, which is still present.
For this specific case, is it safe to consider distributions public?
Yes. Any names exposed by __all__ can be relied upon with the highest confidence. Any non-private names (those without a leading _) may also be fair game, though I would advise against using implementation details like Sectioned. In the past, I've not marked these classes private, but to my chagrin, I've since learned that that's the recommendation.
If that didn't answer your questions, feel free to open a new issue. aiven-anton commented on Dec 6, 2022 @jaraco Thanks for your very thorough response, and sorry for the mixup, this answers all my questions! I didn't notice the reference to the API reference when first reading the docs, now I see it's there, so all good 👍
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|