Skip to content

Conversation

@kurtmckee
Copy link
Contributor

@kurtmckeekurtmckee commented Nov 25, 2024

This PR introduces the following changes, in three commits:

  1. Demonstrate importlib.abc.Traversable.read_text() incompatible with importlib.resources._functional.read_text() usage (Python 3.13) cpython#127012.

    This adds an in-memory finder, loader, and traversable implementation,
    which allows the Traversable protocol and concrete methods to be tested.

    This additional infrastructure demonstrates the original issue,
    but also highlights that the Traversable.joinpath() concrete method
    raises TraversalError which is not getting caught in several places.

  2. Catch TraversalError, raised by Traversable.joinpath().

    Exercising the Traversable protocol's concrete methods
    has highlighted that .joinpath() raises TraversalError,
    which needs to be caught in several places.

    This is primarily resolved within the test suite,
    but implicates the is_resource() function as well.

  3. Resolve a TypeError lurking in the read_text() functional API.

    importlib_resources.read_text() calls the Traversable.read_text()
    concrete method with an errors argument that doesn't exist in the
    method signature, resulting in an TypeError.

    This is resolved by adding an errors parameter to
    Traversable.read_text().

Fixespython/cpython#127012

@kurtmckeekurtmckee changed the title Add a Traversable.readtext()errors parameterAdd a Traversable.read_text()errors parameterNov 25, 2024
@jaracojaraco self-assigned this Jan 3, 2025
This adds an in-memory finder, loader, and traversable implementation, which allows the `Traversable` protocol and concrete methods to be tested. This additional infrastructure demonstrates python/cpython#127012, but also highlights that the `Traversable.joinpath()` concrete method raises `TraversalError` which is not getting caught in several places.
Exercising the `Traversable` protocol's concrete methods has highlighted that `.joinpath()` raises `TraversalError`, which needs to be caught in several places. This is primarily resolved within the test suite, but implicates the `is_resource()` function as well.
`importlib_resources.read_text()` calls the `Traversable.read_text()` concrete method with an `errors` argument that doesn't exist in the method signature, resulting in an `TypeError`. This is resolved by adding an `errors` parameter to `Traversable.read_text()`. Fixespython/cpython#127012
@jaracojaracoforce-pushed the add-readtext-errors-parameter-cpython-issue-127012 branch from 509ac17 to d001110CompareJanuary 3, 2025 16:14
@jaraco
Copy link
Member

jaraco commented Jan 3, 2025

I've force-pushed the three commits after rebasing on main. Now tests are failing because I've re-enabled mypy checks (that were previously disabled).

________________________________________________________ importlib_resources/tests/util.py ________________________________________________________ 279: error: Argument 1 to "reduce" has incompatible type "Callable[[dict[str, object], str], object]"; expected "Callable[[dict[str, object], str], dict[str, object]]" [arg-type] 279: error: Incompatible return value type (got "object", expected "dict[str, object]") [return-value] 284: error: Argument 1 to "reduce" has incompatible type "Callable[[dict[str, object], str], object]"; expected "Callable[[dict[str, object], str], dict[str, object]]" [arg-type] 284: error: Incompatible return value type (got "object", expected "dict[str, object]") [return-value] 

@jaraco
Copy link
Member

In the latest commit, I consolidated the repeated pattern of resolving a fullname from the fixtures, and this incidentally masks the type ambiguity.

@jaracojaracoforce-pushed the add-readtext-errors-parameter-cpython-issue-127012 branch from 30bc726 to 72d550dCompareJanuary 3, 2025 16:38
@jaracojaraco merged commit 10d87bf into python:mainJan 3, 2025
15 checks passed
@jaraco
Copy link
Member

Thanks again for this PR. The work you did here was exemplary - it was clear, understandable, complete, and directly addressed the issue. Bravo.

@kurtmckee
Copy link
ContributorAuthor

kurtmckee commented Jan 4, 2025

Thank you!

@kurtmckeekurtmckee deleted the add-readtext-errors-parameter-cpython-issue-127012 branch January 4, 2025 17:43
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

importlib.abc.Traversable.read_text() incompatible with importlib.resources._functional.read_text() usage (Python 3.13)

2 participants

@kurtmckee@jaraco