Skip to content

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.

Sign up for GitHub

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
from
Open

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

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
2006516
gh-128862: use importlib.resources to acquire doctest resources
graingert Jan 15, 2025
626233e
only compute module relative path when falling back from importlib.re…
graingert Jan 15, 2025
6b30ba8
📜🤖 Added by blurb_it.
blurb-it[bot] Jan 15, 2025
1cb9321
Update Lib/doctest.py
graingert Jan 15, 2025
dca3a82
Merge branch 'main' into use-importlib-resources-for-doctest
graingert Jan 15, 2025
1df5ed7
Merge branch 'main' into use-importlib-resources-for-doctest
graingert Jan 21, 2025
2472168
Merge branch 'main' into use-importlib-resources-for-doctest
graingert Jan 26, 2025
f8f4e63
Merge branch 'main' into use-importlib-resources-for-doctest
graingert Feb 2, 2025
57a37f2
Update Lib/doctest.py
graingert Mar 20, 2025
6a530c4
Merge branch 'main' into use-importlib-resources-for-doctest
graingert Mar 20, 2025
6bda92b
Update doctest.py
graingert Mar 21, 2025
68475b6
Update Lib/doctest.py
AA-Turner Apr 17, 2025
b9ba4e1
Merge branch 'main' into use-importlib-resources-for-doctest
AA-Turner May 1, 2025
0621551
Merge branch 'main' into use-importlib-resources-for-doctest
graingert May 1, 2025
24c06b8
Apply suggestion
AA-Turner May 1, 2025
05f0403
Update 2025-01-15-11-57-42.gh-issue-128862.TDHQdR.rst
AA-Turner May 5, 2025
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 Lib/doctest.py
View file
Open in desktop
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 •
edited
Loading

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
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2025-01-15-11-57-42.gh-issue-128862.TDHQdR.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use ``importlib.resources`` to acquire test files in ``doctest``
Loading