Skip to content
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

pre_commit/util.py:72 read_text / :68 open_binary is deprecated. Use files() instead #2450

Closed
hroncok opened this issue Jul 9, 2022 · 3 comments

Comments

@hroncok
Copy link
Contributor

hroncok commented Jul 9, 2022

search tried in the issue tracker

read_text

describe your issue

When we rebuild pre-commit in Fedora 37 with Python 3.11, 63 tests error with:

________ ERROR at teardown of test_install_uninstall_default_hook_types ________
recwarn = WarningsRecorder(record=True)
    @pytest.fixture(autouse=True)
    def no_warnings(recwarn):
        yield
        warnings = []
        for warning in recwarn:  # pragma: no cover
            message = str(warning.message)
            # ImportWarning: Not importing directory '...' missing __init__(.py)
            if not (
                isinstance(warning.message, ImportWarning) and
                message.startswith('Not importing directory ') and
                ' missing __init__' in message
            ):
                warnings.append(
                    f'{warning.filename}:{warning.lineno} {message}',
                )
>       assert not warnings
E       AssertionError: assert not ['/builddir/build/BUILD/pre-commit-2.19.0/pre_commit/util.py:72 read_text is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.', '/usr/lib64/python3.11/importlib/resources/_legacy.py:80 open_text is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.']
tests/conftest.py:39: AssertionError

Another 4 error with:

__________ ERROR at teardown of test_archive_root_stat[rbenv.tar.gz] ___________
recwarn = WarningsRecorder(record=True)
    @pytest.fixture(autouse=True)
    def no_warnings(recwarn):
        yield
        warnings = []
        for warning in recwarn:  # pragma: no cover
            message = str(warning.message)
            # ImportWarning: Not importing directory '...' missing __init__(.py)
            if not (
                isinstance(warning.message, ImportWarning) and
                message.startswith('Not importing directory ') and
                ' missing __init__' in message
            ):
                warnings.append(
                    f'{warning.filename}:{warning.lineno} {message}',
                )
>       assert not warnings
E       AssertionError: assert not ['/builddir/build/BUILD/pre-commit-2.19.0/pre_commit/util.py:68 open_binary is deprecated. Use files() instead. Refer to https://importlib-resources.readthedocs.io/en/latest/using.html#migrating-from-legacy for migration advice.']
tests/conftest.py:39: AssertionError

pre-commit --version

2.19.0

.pre-commit-config.yaml

not relevant, this is the test suite

~/.cache/pre-commit/pre-commit.log (if present)

No response

@asottile
Copy link
Member

asottile commented Jul 9, 2022

yeah I'm not going to fix this -- I'd recommend disabling the warnings. the full rewrite of importlib.resources is poorly scheduled imo

@asottile asottile closed this as completed Jul 9, 2022
@hroncok
Copy link
Contributor Author

hroncok commented Jul 10, 2022

I'd recommend disabling the warnings.

I've tried to play with PYTHONWARNINGS and no luck. The only way I could get this to pass was to adapt the no_warnings fixture. Would that be accepted here, o should we carry it downstream only?

diff --git a/tests/conftest.py b/tests/conftest.py
index b68a1d0..b04680b 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -28,11 +28,14 @@ def no_warnings(recwarn):
     for warning in recwarn:  # pragma: no cover
         message = str(warning.message)
         # ImportWarning: Not importing directory '...' missing __init__(.py)
-        if not (
+        if not ((
             isinstance(warning.message, ImportWarning) and
             message.startswith('Not importing directory ') and
             ' missing __init__' in message
-        ):
+        ) or (
+            isinstance(warning.message, DeprecationWarning) and
+            '#migrating-from-legacy' in message
+        )):
             warnings.append(
                 f'{warning.filename}:{warning.lineno} {message}',
             )

@asottile
Copy link
Member

I'll just remove the warnings fixture entirely -- #2454

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants