gh-128862: use importlib.resources to acquire doctest resources

[graingert]

• Issue: doctest._load_testfile uses deprecated Loader.get_data API, it should use importlib.resources #128862

[serhiy-storchaka]

What exceptions should be suppressed?

Exception is too wide class. It includes OSError, UnicodeDecodingError, MemoryError, which currently are not suppressed.

[AA-Turner]

This control flow feels odd, because we're returning in a suppress context manager.

Perhaps:

text = None
if module_relative:
    package = _normalize_module(package, depth=3)
    try:
        file = importlib.resources.files(package) / filename
        text = file.read_text(encoding=encoding)
    except AttributeError:
        filename = _module_relative_path(package, filename)

if text is None:
    with open(filename, encoding=encoding) as f:
        text = f.read()

return text, filename

[AA-Turner]

cc @graingert, are you able to get to this before Tuesday (feature freeze)?

A

[graingert]

I think @jaraco has taken over this PR

[AA-Turner]

I've applied my suggestion, narrowing from Exception to AttributeError, per Jason above:

Even better would be if importlib.resources could throw a more specific exception when the indicated module is unable to resolve resources, but since it currently just raises an AttributeError, perhaps it's best to just trap that exception, rather than try to pre-emptively detect that exception will occur.

[serhiy-storchaka]

Do not suppress arbitrary Exception, suppress only the necessary.

Also, I do not think that it is worth to import contextlib for a simple except.

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[graingert]

thanks!

[serhiy-storchaka]

Is it raised by importlib.resources.files() or by file.read_text()? Can text be None?

[brettcannon]

@graingert FYI your PR has merge conflicts and you have not left a reply with the magic sentence to tell Bedevere that you're ready for a review.

[github-actions]

This PR is stale because it has been open for 30 days with no activity.