chore: fix review comments

Author: erikwrede

.gitignore

@@ -12,6 +12,7 @@ __pycache__/
 .Python
 env/
 .venv/
+venv/
 build/
 develop-eggs/
 dist/

graphene_sqlalchemy/tests/models.py

@@ -16,12 +16,12 @@
     String,
     Table,
     func,
-    select,
 )
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.orm import backref, column_property, composite, mapper, relationship
 
+from graphene_sqlalchemy.tests.utils import wrap_select_func
 from graphene_sqlalchemy.utils import SQL_VERSION_HIGHER_EQUAL_THAN_1_4
 
 PetKind = Enum("cat", "dog", name="pet_kind")
@@ -118,15 +118,9 @@ def hybrid_prop_bool(self) -> bool:
     def hybrid_prop_list(self) -> List[int]:
         return [1, 2, 3]
 
-    # TODO Remove when switching min sqlalchemy version to SQLAlchemy 1.4
-    if SQL_VERSION_HIGHER_EQUAL_THAN_1_4:
-        column_prop = column_property(
-            select(func.cast(func.count(id), Integer)), doc="Column property"
-        )
-    else:
-        column_prop = column_property(
-            select([func.cast(func.count(id), Integer)]), doc="Column property"
-        )
+    column_prop = column_property(
+        wrap_select_func(func.cast(func.count(id), Integer)), doc="Column property"
+    )
 
     composite_prop = composite(
         CompositeFullName, first_name, last_name, doc="Composite"

graphene_sqlalchemy/tests/models_batching.py

@@ -11,12 +11,11 @@
     String,
     Table,
     func,
-    select,
 )
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.orm import column_property, relationship
 
-from graphene_sqlalchemy.utils import SQL_VERSION_HIGHER_EQUAL_THAN_1_4
+from graphene_sqlalchemy.tests.utils import wrap_select_func
 
 PetKind = Enum("cat", "dog", name="pet_kind")
 
@@ -62,14 +61,9 @@ class Reporter(Base):
     articles = relationship("Article", backref="reporter")
     favorite_article = relationship("Article", uselist=False)
 
-    if SQL_VERSION_HIGHER_EQUAL_THAN_1_4:
-        column_prop = column_property(
-            select(func.cast(func.count(id), Integer)), doc="Column property"
-        )
-    else:
-        column_prop = column_property(
-            select([func.cast(func.count(id), Integer)]), doc="Column property"
-        )
+    column_prop = column_property(
+        wrap_select_func(func.cast(func.count(id), Integer)), doc="Column property"
+    )
 
 
 class Article(Base):

graphene_sqlalchemy/tests/test_converter.py

@@ -2,19 +2,26 @@
 import sys
 from typing import Dict, Union
 
+import graphene
 import pytest
 import sqlalchemy_utils as sqa_utils
-from sqlalchemy import Column, func, select, types
+from graphene.relay import Node
+from graphene.types.structures import Structure
+from sqlalchemy import Column, func, types
 from sqlalchemy.dialects import postgresql
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy.ext.hybrid import hybrid_property
 from sqlalchemy.inspection import inspect
 from sqlalchemy.orm import column_property, composite
 
-import graphene
-from graphene.relay import Node
-from graphene.types.structures import Structure
-
+from .models import (
+    Article,
+    CompositeFullName,
+    Pet,
+    Reporter,
+    ShoppingCart,
+    ShoppingCartItem,
+)
 from ..converter import (
     convert_sqlalchemy_column,
     convert_sqlalchemy_composite,
@@ -24,15 +31,7 @@
 from ..fields import UnsortedSQLAlchemyConnectionField, default_connection_field_factory
 from ..registry import Registry, get_global_registry
 from ..types import ORMField, SQLAlchemyObjectType
-from ..utils import SQL_VERSION_HIGHER_EQUAL_THAN_1_4, is_sqlalchemy_version_less_than
-from .models import (
-    Article,
-    CompositeFullName,
-    Pet,
-    Reporter,
-    ShoppingCart,
-    ShoppingCartItem,
-)
+from ..utils import is_sqlalchemy_version_less_than
 
 
 def mock_resolver():
@@ -88,9 +87,9 @@ def prop_method() -> int | str:
         return "not allowed in gql schema"
 
     with pytest.raises(
-        ValueError,
-        match=r"Cannot convert hybrid_property Union to "
-        r"graphene.Union: the Union contains scalars. \.*",
+            ValueError,
+            match=r"Cannot convert hybrid_property Union to "
+                  r"graphene.Union: the Union contains scalars. \.*",
     ):
         get_hybrid_property_type(prop_method)
 
@@ -337,25 +336,9 @@ class TestEnum(enum.IntEnum):
     assert graphene_type._meta.enum.__members__["two"].value == 2
 
 
-@pytest.mark.skipif(
-    not SQL_VERSION_HIGHER_EQUAL_THAN_1_4,
-    reason="SQLAlchemy <1.4 does not support this",
-)
-def test_should_columproperty_convert_sqa_20():
-    field = get_field_from_column(
-        column_property(select(func.sum(func.cast(id, types.Integer))).where(id == 1))
-    )
-
-    assert field.type == graphene.Int
-
-
-@pytest.mark.skipif(
-    not is_sqlalchemy_version_less_than("2.0.0b1"),
-    reason="SQLAlchemy >=2.0 does not support this syntax, see convert_sqa_20",
-)
 def test_should_columproperty_convert():
     field = get_field_from_column(
-        column_property(select([func.sum(func.cast(id, types.Integer))]).where(id == 1))
+        column_property(wrap_select_func(func.sum(func.cast(id, types.Integer))).where(id == 1))
     )
 
     assert field.type == graphene.Int
@@ -654,8 +637,8 @@ class Meta:
     )
 
     for (
-        hybrid_prop_name,
-        hybrid_prop_expected_return_type,
+            hybrid_prop_name,
+            hybrid_prop_expected_return_type,
     ) in shopping_cart_item_expected_types.items():
         hybrid_prop_field = ShoppingCartItemType._meta.fields[hybrid_prop_name]
 
@@ -666,7 +649,7 @@ class Meta:
             str(hybrid_prop_expected_return_type),
         )
         assert (
-            hybrid_prop_field.description is None
+                hybrid_prop_field.description is None
         )  # "doc" is ignored by hybrid property
 
     ###################################################
@@ -714,8 +697,8 @@ class Meta:
     )
 
     for (
-        hybrid_prop_name,
-        hybrid_prop_expected_return_type,
+            hybrid_prop_name,
+            hybrid_prop_expected_return_type,
     ) in shopping_cart_expected_types.items():
         hybrid_prop_field = ShoppingCartType._meta.fields[hybrid_prop_name]
 
@@ -726,5 +709,5 @@ class Meta:
             str(hybrid_prop_expected_return_type),
         )
         assert (
-            hybrid_prop_field.description is None
+                hybrid_prop_field.description is None
         )  # "doc" is ignored by hybrid property

graphene_sqlalchemy/tests/utils.py

@@ -1,6 +1,10 @@
 import inspect
 import re
 
+from sqlalchemy import select
+
+from graphene_sqlalchemy.utils import SQL_VERSION_HIGHER_EQUAL_THAN_1_4
+
 
 def to_std_dicts(value):
     """Convert nested ordered dicts to normal dicts for better comparison."""
@@ -18,8 +22,15 @@ def remove_cache_miss_stat(message):
     return re.sub(r"\[generated in \d+.?\d*s\]\s", "", message)
 
 
-async def eventually_await_session(session, func, *args):
+def wrap_select_func(query):
+    # TODO remove this when we drop support for sqa < 2.0
+    if SQL_VERSION_HIGHER_EQUAL_THAN_1_4:
+        return select(query)
+    else:
+        return select([query])
+
 
+async def eventually_await_session(session, func, *args):
     if inspect.iscoroutinefunction(getattr(session, func)):
         await getattr(session, func)(*args)
     else: