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

Python 3.10 raises ModuleNotFoundError in cattrs/_compat.py #369

Closed
idwagner opened this issue May 30, 2023 · 16 comments · Fixed by #370
Closed

Python 3.10 raises ModuleNotFoundError in cattrs/_compat.py #369

idwagner opened this issue May 30, 2023 · 16 comments · Fixed by #370

Comments

@idwagner
Copy link

  • cattrs version: 23.1.0
  • Python version: 3.10.4
  • Operating System: linux

Description

In python 3.10, the import of cattrs/_compat is throwing a ModuleNotFoundError.

What I Did

An automated pipeline we are using started throwing a ModuleNotFoundError today. This appears to be related to #364, where the import is not catching the correct exceptions for importing typing_extensions.

Traceback (most recent call last):
  File "/builddir/.venv/bin/REDACTED", line 5, in <module>
    from REDACTED import main
  File "/builddir/.venv/lib/python3.10/site-packages/REDACTED/__init__.py", line 4, in <module>
    from requests_cache import CachedSession, DO_NOT_CACHE
  File "/builddir/.venv/lib/python3.10/site-packages/requests_cache/__init__.py", line 7, in <module>
    from .backends import *
  File "/builddir/.venv/lib/python3.10/site-packages/requests_cache/backends/__init__.py", line 7, in <module>
    from .base import BaseCache, BaseStorage, DictStorage
  File "/builddir/.venv/lib/python3.10/site-packages/requests_cache/backends/base.py", line 22, in <module>
    from ..serializers import SerializerType, init_serializer
  File "/builddir/.venv/lib/python3.10/site-packages/requests_cache/serializers/__init__.py", line 24, in <module>
    from .cattrs import CattrStage
  File "/builddir/.venv/lib/python3.10/site-packages/requests_cache/serializers/cattrs.py", line 19, in <module>
    from cattr import Converter
  File "/builddir/.venv/lib/python3.10/site-packages/cattr/__init__.py", line 1, in <module>
    from .converters import BaseConverter, Converter, GenConverter, UnstructureStrategy
  File "/builddir/.venv/lib/python3.10/site-packages/cattr/converters.py", line 1, in <module>
    from cattrs.converters import (
  File "/builddir/.venv/lib/python3.10/site-packages/cattrs/__init__.py", line 1, in <module>
    from .converters import BaseConverter, Converter, GenConverter, UnstructureStrategy
  File "/builddir/.venv/lib/python3.10/site-packages/cattrs/converters.py", line 26, in <module>
    from ._compat import (
  File "/builddir/.venv/lib/python3.10/site-packages/cattrs/_compat.py", line 372, in <module>
    from typing_extensions import NotRequired, Required
ModuleNotFoundError: No module named 'typing_extensions'```
@NeonDaniel
Copy link

Seeing the same error in this automation run

@jdennis
Copy link

jdennis commented May 30, 2023

I'm hitting the same error, here's the root cause of the problem:

In https://github.com/python-attrs/cattrs/blob/00932450fb27d8c6a53697244ae033b13916489b/src/cattrs/_compat.py#L369-#L372
It's clear typing_extensions is still required for py 3.10, but typing_extensions is excluded from the project dependencies in pyproject.toml for py >= 3.10 as seen here:

typing_extensions = { version = "*", python = "< 3.10" }

@henryiii
Copy link
Contributor

FYI, a quick fix (besides restricting 23.1.0 exactly, which there's already an example of above) is to add the transitive dependency, like in scikit-build/scikit-build-core@4b6dcfb.

@Tinche
Copy link
Member

Tinche commented May 30, 2023

Thanks, I'll put out a 23.1.1 with this fix. I guess one of our dev dependencies pulled in typing_extensions so the bug was masked in the test harness.

This is an easy fix, but a more troubling issue is - how do we prevent stuff like this in the future? :/

@Tinche
Copy link
Member

Tinche commented May 30, 2023

23.1.1 has been published to PyPI, thanks everyone.

@Tinche
Copy link
Member

Tinche commented May 30, 2023

I might as well yank 23.1.0, right?

@henryiii
Copy link
Contributor

Won't hurt to yank.

@Tinche
Copy link
Member

Tinche commented May 30, 2023

Yanked.

@henryiii
Copy link
Contributor

how do we prevent stuff like this in the future?

This is an excellent question, and I'm not sure I have a great answer.

Probably not on try/except blocks, but it does seem like static version comparisons blocks could be validated by a tool, but I don't know of one (and if there was one, it probably would be based on PEP 621, not Poetry 1's custom format). Things like mypy can validate the correct version settings, but they can't verify that you are requiring the correct dependencies - mypy often uses stubs, so it's not even the same list.

For a dynamic check, running an "example" as an integration test that doesn't have testing deps, but just runs some code to see if it passes, with no extra dependencies, would help catch this sort of thing.

You can also make a "downstream" tester, like ruff & mypy do. I've done that via a manual nox job in scikit-build/scikit-build-core. It's different enough for build systems that you'd probably not get much out of starting from that, though.

@musicinmybrain
Copy link

I am not sure what I should expect to see, or if this is the right place to talk about it, but I was looking at what updating to 23.1.0 on Fedora Rawhide would look like, and I encountered quite a few test failures with Python 3.11.3,

=========================== short test summary info ============================
FAILED tests/test_converter.py::test_annotated_with_typing_extensions_attrs
FAILED tests/test_typeddicts.py::test_simple_roundtrip - TypeError: 'NoneType...
FAILED tests/test_typeddicts.py::test_simple_nontotal - TypeError: 'NoneType'...
FAILED tests/test_typeddicts.py::test_int_override - TypeError: 'NoneType' ob...
FAILED tests/test_typeddicts.py::test_extra_keys - TypeError: 'NoneType' obje...
FAILED tests/test_typeddicts.py::test_not_required - TypeError: 'NoneType' ob...
FAILED tests/test_typeddicts.py::test_required - TypeError: 'NoneType' object...
FAILED tests/test_typeddicts.py::test_omit - TypeError: 'NoneType' object is ...
FAILED tests/test_typeddicts.py::test_rename - TypeError: 'NoneType' object i...
FAILED tests/test_typeddicts.py::test_forbid_extra_keys - TypeError: 'NoneTyp...
====== 10 failed, 359 passed, 1 skipped, 15 xfailed in 196.16s (0:03:16) =======

One of these is:

_________________ test_annotated_with_typing_extensions_attrs __________________

    def test_annotated_with_typing_extensions_attrs():
        """Annotation support works for attrs classes."""
>       from typing_extensions import Annotated
E       ModuleNotFoundError: No module named 'typing_extensions'

tests/test_converter.py:660: ModuleNotFoundError

The rest are something like:

____________________________ test_simple_roundtrip _____________________________

    @given(simple_typeddicts(typeddict_cls=None if not is_py38 else ExtensionsTypedDict))
>   def test_simple_roundtrip(cls_and_instance) -> None:

f          = <function given.<locals>.run_test_as_given.<locals>.wrapped_test at 0x7f5ae6849940>

tests/test_typeddicts.py:61:  
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

draw = <bound method ConjectureData.draw of ConjectureData(VALID, 3 bytes, frozen)>
total = False, not_required = False, min_attrs = 0, typeddict_cls = None

    @composite
    def simple_typeddicts(
        draw: DrawFn,
        total: Optional[bool] = None,
        not_required: bool = False,
        min_attrs: int = 0,   
        typeddict_cls: Optional[Any] = None,
    ) -> Tuple[TypedDictType, dict]:
        """Generate simple typed dicts.

        :param total: Generate the given totality dicts (default = random)
        """
        if total is None:
            total = draw(booleans())

        attrs = draw(
            lists(
                int_attributes(total, not_required)
                | list_of_int_attributes(total, not_required)
                | datetime_attributes(total, not_required),
                min_size=min_attrs,
            )
        )

        attrs_dict = {n: attr[0] for n, attr in zip(gen_attr_names(), attrs)}
        success_payload = {}  
        for n, a in zip(attrs_dict, attrs):
            v = draw(a[1])
            if v is not NOTHING:
                success_payload[n] = v

        if typeddict_cls is None:
>           cls = (TypedDict if draw(booleans()) else ExtensionsTypedDict)(
                "HypTypedDict", attrs_dict, total=total
            )
E           TypeError: 'NoneType' object is not callable

attrs      = []
attrs_dict = {}
draw       = <bound method ConjectureData.draw of ConjectureData(VALID, 3 bytes, frozen)>
min_attrs  = 0
not_required = False
success_payload = {}
total      = False
typeddict_cls = None

tests/typeddicts.py:128: TypeError

All of these test failures disappear if I patch pyproject.toml to require typing-extensions on all Python interpreter versions.

@musicinmybrain
Copy link

23.1.1 has been published to PyPI, thanks everyone.

Could you please push the git tag? Thanks!

@Tinche
Copy link
Member

Tinche commented May 30, 2023

23.1.1 has been published to PyPI, thanks everyone.

Could you please push the git tag? Thanks!

Tag pushed, whoops

@Tinche
Copy link
Member

Tinche commented May 30, 2023

I am not sure what I should expect to see, or if this is the right place to talk about it, but I was looking at what updating to 23.1.0 on Fedora Rawhide would look like, and I encountered quite a few test failures with Python 3.11.3,

All of these test failures disappear if I patch pyproject.toml to require typing-extensions on all Python interpreter versions.

Howdy,

let's open a new issue for this. How are you setting up your test environment? For me, typing_extensions 4.5.0 gets brought in transitively.

@jdennis
Copy link

jdennis commented May 31, 2023

This is an easy fix, but a more troubling issue is - how do we prevent stuff like this in the future? :/

One of the Python test harnesses (perhaps more that one) allow you to iterate the test over multiple Python versions. Sorry but I can't remember which test harness that is (I've used too many), maybe someone else recalls.

@musicinmybrain
Copy link

Howdy,

let's open a new issue for this. How are you setting up your test environment? For me, typing_extensions 4.5.0 gets brought in transitively.

Well, this is the RPM build environment for Fedora Linux, so we’re not letting tox install all the dev dependencies specified in tool.poetry.group.dev.dependencies, because:

  • Some are for linting/coverage/formatting/etc. and do not make sense to require downstream just for running tests in the distribution, and our guidelines encourage not bringing these in.
  • Some are for rendering the Sphinx documentation as HTML, which we don’t do for various reasons.

Indeed, tox -e py311 works fine on a fresh checkout of this repo.

So if typing_extensions should really be considered just a test dependency on Python 3.11+, and you don’t care that it is normally brought in transitively and implicitly rather than specified explicitly, then I suppose there’s nothing to change here. We (Fedora packagers) just need to make sure we name it as an explicit build-time dependency for testing, since we aren’t pulling in the whole panoply of “dev” dependencies.

@Tinche
Copy link
Member

Tinche commented May 31, 2023

@musicinmybrain Yeah, the idea is you don't need typing_extensions on 3.11 in a non-dev context. Specifically, this version of cattrs depends on typing_extensions more because of TypedDicts, but all parts required for that are in typing on 3.11. So I'd just add typing_extensions to the dev deps on your end.

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

Successfully merging a pull request may close this issue.

6 participants