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.13 test failure: tests/config/loader/test_str_convert.py::test_str_convert_ok_py39[1,2-value1-Optional] - TypeError: issubclass() arg 1 must be a class #3290

Open
mgorny opened this issue Jun 7, 2024 · 4 comments

Comments

@mgorny
Copy link
Contributor

mgorny commented Jun 7, 2024

Issue

When running the test suite on Python 3.13.0b1, I'm getting the following test failure:

____________________________________________ test_str_convert_ok_py39[1,2-value1-Optional] ____________________________________________

raw = '1,2', value = ['1', '2'], of_type = typing.Optional[list[str]]

    @pytest.mark.parametrize(
        ("raw", "value", "of_type"),
        [
            ("", None, Optional[list[str]]),
            ("1,2", ["1", "2"], Optional[list[str]]),
        ],
    )
    def test_str_convert_ok_py39(raw: str, value: Any, of_type: type[Any]) -> None:
>       result = StrConvert().to(raw, of_type, None)

of_type    = typing.Optional[list[str]]
raw        = '1,2'
value      = ['1', '2']

tests/config/loader/test_str_convert.py:71: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../tox-4.15.1-python3_13/install/usr/lib/python3.13/site-packages/tox/config/loader/convert.py:32: in to
    return self._to_typing(raw, of_type, factory)
        factory    = None
        from_module = 'typing'
        of_type    = typing.Optional[list[str]]
        raw        = '1,2'
        self       = <tox.config.loader.str_convert.StrConvert object at 0x7fdd060385a0>
../tox-4.15.1-python3_13/install/usr/lib/python3.13/site-packages/tox/config/loader/convert.py:79: in _to_typing
    result = self.to(raw, new_type, factory)
        args       = (list[str], <class 'NoneType'>)
        factory    = None
        new_type   = list[str]
        none       = <class 'NoneType'>
        of_type    = typing.Optional[list[str]]
        origin     = typing.Union
        raw        = '1,2'
        result     = <object object at 0x7fdd090df730>
        self       = <tox.config.loader.str_convert.StrConvert object at 0x7fdd060385a0>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <tox.config.loader.str_convert.StrConvert object at 0x7fdd060385a0>, raw = '1,2', of_type = list[str], factory = None

    def to(self, raw: T, of_type: type[V], factory: Factory[V]) -> V:  # noqa: PLR0911
        """
        Convert given raw type to python type.
    
        :param raw: the raw type
        :param of_type: python type
        :param factory: factory method to build the object
        :return: the converted type
        """
        from_module = getattr(of_type, "__module__", None)
        if from_module in {"typing", "typing_extensions"}:
            return self._to_typing(raw, of_type, factory)
>       if issubclass(of_type, Path):
E       TypeError: issubclass() arg 1 must be a class

factory    = None
from_module = 'builtins'
of_type    = list[str]
raw        = '1,2'
self       = <tox.config.loader.str_convert.StrConvert object at 0x7fdd060385a0>

../tox-4.15.1-python3_13/install/usr/lib/python3.13/site-packages/tox/config/loader/convert.py:33: TypeError

Apparently, this is intentional change: python/cpython#101162

Environment

Provide at least:

  • OS: Gentoo Linux amd64

Output of running tox

n/a

Minimal example

n/a

@hroncok
Copy link
Contributor

hroncok commented Jul 28, 2024

This ugly diff makes it work:

diff --git a/src/tox/config/loader/convert.py b/src/tox/config/loader/convert.py
index 9379c6ea..66ad266d 100644
--- a/src/tox/config/loader/convert.py
+++ b/src/tox/config/loader/convert.py
@@ -30,6 +30,10 @@ class Convert(ABC, Generic[T]):
         from_module = getattr(of_type, "__module__", None)
         if from_module in {"typing", "typing_extensions"}:
             return self._to_typing(raw, of_type, factory)
+        # python does not allow use of parametrized generics with isinstance,
+        # so we need to check for them.
+        if hasattr(typing, "GenericAlias") and isinstance(of_type, typing.GenericAlias):
+            return list(self.to_list(raw, of_type=of_type))  # type: ignore[return-value]
         if issubclass(of_type, Path):
             return self.to_path(raw)  # type: ignore[return-value]
         if issubclass(of_type, bool):
@@ -40,10 +44,6 @@ class Convert(ABC, Generic[T]):
             return self.to_env_list(raw)  # type: ignore[return-value]
         if issubclass(of_type, str):
             return self.to_str(raw)  # type: ignore[return-value]
-        # python does not allow use of parametrized generics with isinstance,
-        # so we need to check for them.
-        if hasattr(typing, "GenericAlias") and isinstance(of_type, typing.GenericAlias):
-            return list(self.to_list(raw, of_type=of_type))  # type: ignore[return-value]
         if isinstance(raw, of_type):  # already target type no need to transform it
             # do it this late to allow normalization - e.g. string strip
             return raw
diff --git a/src/tox/config/loader/str_convert.py b/src/tox/config/loader/str_convert.py
index 63fc41de..f2888a13 100644
--- a/src/tox/config/loader/str_convert.py
+++ b/src/tox/config/loader/str_convert.py
@@ -6,6 +6,7 @@ import shlex
 import sys
 from itertools import chain
 from pathlib import Path
+import typing
 from typing import TYPE_CHECKING, Any, Iterator
 
 from tox.config.loader.convert import Convert
@@ -28,7 +29,7 @@ class StrConvert(Convert[str]):
 
     @staticmethod
     def to_list(value: str, of_type: type[Any]) -> Iterator[str]:
-        splitter = "\n" if issubclass(of_type, Command) or "\n" in value else ","
+        splitter = "\n" if (not isinstance(of_type, getattr(typing, "GenericAlias", type(None))) and issubclass(of_type, Command)) or "\n" in value else ","
         splitter = splitter.replace("\r", "")
         for token in value.split(splitter):
             value = token.strip()

It makes sure that:

  • the GenericAlias check in convert.py happens first
  • the GenericAlias check is added to str_convert.py

The change in str_convert could probably be split into a function of its own.

@hroncok
Copy link
Contributor

hroncok commented Jul 29, 2024

I'd submit a PR, but I wonder where else this is used, possibly not covered by tests?

@hroncok
Copy link
Contributor

hroncok commented Jul 29, 2024

Also, if I need a is_generic_alias(t) function, where would be the best place for it to be defined?

@gaborbernat
Copy link
Member

Up to you 👍 I would keep it private and close to where it is used.

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

No branches or pull requests

3 participants