Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 16 commits into
base: main
Choose a base branch
Loading
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 9 additions & 16 deletions 25 Lib/doctest.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def _test():
]

import __future__
import contextlib
import difflib
import inspect
import linecache
Expand All @@ -102,7 +103,8 @@ def _test():
import sys
import traceback
import unittest
from io import StringIO, IncrementalNewlineDecoder
import importlib.resources
from io import StringIO
from collections import namedtuple
import _colorize # Used in doctests
from _colorize import ANSIColors, can_colorize
Expand Down Expand Up @@ -235,25 +237,16 @@ def _normalize_module(module, depth=2):
else:
raise TypeError("Expected a module, string, or None")

def _newline_convert(data):
# The IO module provides a handy decoder for universal newline conversion
return IncrementalNewlineDecoder(None, True).decode(data, True)

def _load_testfile(filename, package, module_relative, encoding):
if module_relative:
package = _normalize_module(package, 3)
with contextlib.suppress(Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exceptions should be suppressed?

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

Copy link
Member

@AA-Turner AA-Turner Apr 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

A

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think @jaraco has taken over this PR

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

text = importlib.resources.read_text(package, filename,
encoding=encoding)
return text, filename

filename = _module_relative_path(package, filename)
if (loader := getattr(package, '__loader__', None)) is None:
try:
loader = package.__spec__.loader
except AttributeError:
pass
if hasattr(loader, 'get_data'):
file_contents = loader.get_data(filename)
file_contents = file_contents.decode(encoding)
# get_data() opens files as 'rb', so one must do the equivalent
# conversion as universal newlines would do.
return _newline_convert(file_contents), filename

with open(filename, encoding=encoding) as f:
return f.read(), filename

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use ``importlib.resources`` to acquire test files in ``doctest``
AA-Turner marked this conversation as resolved.
Show resolved Hide resolved
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.