From 02cb4a3d647c92773496f1bd1262748a56895ef6 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 11:51:28 -0500 Subject: [PATCH 01/10] Get mypy passing --- .pre-commit-config.yaml | 11 +++++ pynocular/aiopg_transaction.py | 18 +++++--- pynocular/database_model.py | 69 ++++++++++++++++++++---------- pynocular/exceptions.py | 9 ++-- pynocular/nested_database_model.py | 7 ++- pynocular/patch_models.py | 22 +++++----- setup.cfg | 1 + 7 files changed, 94 insertions(+), 43 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 164f5df..1edac0f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -80,6 +80,17 @@ repos: - "flake8-import-order<0.19,>=0.18" - flake8-print>=3.1.4,<4 + - id: mypy + name: Type check Python (mypy) + entry: mypy -p pynocular + language: python + types: [file, python] + additional_dependencies: + - mypy==0.931 + - types-dataclasses==0.6.1 + - pydantic~=1.9.0 + pass_filenames: false + - id: circleci-config name: Pack the CircleCI config entry: .githooks/pre-commit-scripts/circleci.sh diff --git a/pynocular/aiopg_transaction.py b/pynocular/aiopg_transaction.py index ac04d15..1e3010d 100644 --- a/pynocular/aiopg_transaction.py +++ b/pynocular/aiopg_transaction.py @@ -1,7 +1,7 @@ """Module for aiopg transaction utils""" import asyncio import sys -from typing import Dict, Optional, Union +from typing import Any, Dict, no_type_check, Optional, Type, Union import aiocontextvars as contextvars from aiopg.sa.connection import SAConnection @@ -10,6 +10,7 @@ transaction_connections_var = contextvars.ContextVar("transaction_connections") +@no_type_check def get_current_task() -> asyncio.Task: """Get the current task when this method is called @@ -37,12 +38,12 @@ def __init__(self, connection: SAConnection) -> None: self._conn = connection self.lock = asyncio.Lock() - async def execute(self, *args, **kwargs): + async def execute(self, *args: Any, **kwargs: Any) -> Any: """Wrapper around the `execute` method of the wrapped SAConnection""" async with self.lock: return await self._conn.execute(*args, **kwargs) - def __getattr__(self, attr): + def __getattr__(self, attr: str) -> Any: """Except for execute, all other attributes should pass through""" return getattr(self._conn, attr) @@ -192,7 +193,10 @@ async def __aenter__(self) -> LockedConnection: raise return conn - async def __aexit__(self, exc_type, exc_value, tb) -> None: + @no_type_check + async def __aexit__( + self, exc_type: Optional[Type], exc_value: Any, tb: Any + ) -> None: """Exit the transaction context If this is the top level context, then commit the transaction (unless @@ -251,7 +255,7 @@ def __init__(self, engine: aiopg.sa.engine.Engine) -> None: """ super().__init__(engine) # The connection object, if functioning as standard connection - self._conn = None + self._conn: Optional[SAConnection] = None async def __aenter__(self) -> Union[LockedConnection, SAConnection]: """Conditionally establish the transaction context @@ -269,7 +273,9 @@ async def __aenter__(self) -> Union[LockedConnection, SAConnection]: self._conn = await self._engine.acquire() return self._conn - async def __aexit__(self, exc_type, exc_value, tb) -> None: + async def __aexit__( + self, exc_type: Optional[Type], exc_value: Any, tb: Any + ) -> None: """Exit the transaction context""" if self._conn is not None: await self._conn.close() diff --git a/pynocular/database_model.py b/pynocular/database_model.py index 24bd890..f0cd173 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -3,7 +3,20 @@ from datetime import datetime from enum import Enum, EnumMeta import inspect -from typing import Any, Callable, Dict, Generator, List, Optional, Sequence, Set, Union +from typing import ( + Any, + Callable, + cast, + Dict, + Generator, + List, + Optional, + Sequence, + Set, + Type, + TypeVar, + Union, +) from uuid import UUID as stdlib_uuid from aenum import Enum as AEnum, EnumMeta as AEnumMeta @@ -25,6 +38,7 @@ from sqlalchemy.schema import FetchedValue from sqlalchemy.sql.base import ImmutableColumnCollection from sqlalchemy.sql.elements import BinaryExpression, UnaryExpression +from typing_extensions import TypeGuard from pynocular.engines import DBEngine, DBInfo from pynocular.exceptions import ( @@ -39,18 +53,18 @@ from pynocular.nested_database_model import NestedDatabaseModel -def is_valid_uuid(string: str) -> bool: - """Check if a string is a valid UUID +def is_valid_uuid(obj: Any) -> TypeGuard["UUID_STR"]: + """Check if an object is a valid UUID Args: - string: the string to check + obj: the object to check Returns: Whether or not the string is a well-formed UUIDv4 """ try: - stdlib_uuid(string, version=4) + stdlib_uuid(obj, version=4) return True except (TypeError, AttributeError, ValueError): return False @@ -72,14 +86,14 @@ def validate(cls, v: Any) -> str: v: The value to validate """ - if isinstance(v, stdlib_uuid) or (isinstance(v, str) and is_valid_uuid(v)): + if isinstance(v, stdlib_uuid) or is_valid_uuid(v): return str(v) else: raise ValueError("invalid UUID string") def nested_model( - db_model_class: "DatabaseModel", reference_field: str = None + db_model_class: Type["DatabaseModel"], reference_field: str = None ) -> Callable: """Generate a NestedModel class with dynamic model references @@ -109,12 +123,16 @@ def validate(cls, v: Union[UUID_STR, "DatabaseModel"]) -> NestedDatabaseModel: if is_valid_uuid(v): return NestedDatabaseModel(db_model_class, v) else: + v = cast("DatabaseModel", v) return NestedDatabaseModel(db_model_class, v.get_primary_id(), v) return NestedModel -def database_model(table_name: str, database_info: DBInfo) -> "DatabaseModel": +T = TypeVar("T", bound=Type[BaseModel]) + + +def database_model(table_name: str, database_info: DBInfo) -> Callable[[T], T]: """Decorator that adds SQL functionality to Pydantic BaseModel objects Args: @@ -128,21 +146,24 @@ def database_model(table_name: str, database_info: DBInfo) -> "DatabaseModel": """ - def wrapped(cls): + def wrapped(cls: T) -> T: if BaseModel not in inspect.getmro(cls): raise DatabaseModelMisconfigured( "Model is not subclass of pydantic.BaseModel" ) cls.__bases__ += (DatabaseModel,) - cls.initialize_table(table_name, database_info) + cls.initialize_table(table_name, database_info) # type: ignore return cls return wrapped -class DatabaseModel: +SelfType = TypeVar("SelfType", bound="DatabaseModel") + + +class DatabaseModel(BaseModel): """Adds database functionality to a Pydantic BaseModel A DatabaseModel is a Pydantic based model along with a SQLAlchemy @@ -302,7 +323,7 @@ async def get_with_refs(cls, *args: Any, **kwargs: Any) -> "DatabaseModel": return obj @classmethod - async def get(cls, *args: Any, **kwargs: Any) -> "DatabaseModel": + async def get(cls: Type[SelfType], *args: Any, **kwargs: Any) -> SelfType: """Gets the DatabaseModel for the given primary key value(s) Args: @@ -344,7 +365,7 @@ async def get(cls, *args: Any, **kwargs: Any) -> "DatabaseModel": return records[0] @classmethod - async def get_list(cls, **kwargs: Any) -> List["DatabaseModel"]: + async def get_list(cls: Type[SelfType], **kwargs: Any) -> List[SelfType]: """Fetches the DatabaseModel for based on the provided kwargs Args: @@ -380,11 +401,11 @@ async def get_list(cls, **kwargs: Any) -> List["DatabaseModel"]: @classmethod async def select( - cls, + cls: Type[SelfType], where_expressions: Optional[List[BinaryExpression]] = None, order_by: Optional[List[UnaryExpression]] = None, limit: Optional[int] = None, - ) -> List["DatabaseModel"]: + ) -> List[SelfType]: """Execute a SELECT on the DatabaseModel table with the given parameters Args: @@ -421,7 +442,7 @@ async def select( return [cls.from_dict(dict(record)) for record in records] @classmethod - async def create(cls, **data) -> "DatabaseModel": + async def create(cls: Type[SelfType], **data: Dict[str, Any]) -> SelfType: """Create a new instance of the this DatabaseModel and save it Args: @@ -437,7 +458,9 @@ async def create(cls, **data) -> "DatabaseModel": return new @classmethod - async def create_list(cls, models: List["DatabaseModel"]) -> List["DatabaseModel"]: + async def create_list( + cls: Type[SelfType], models: List[SelfType] + ) -> List[SelfType]: """Create new batch of records in one query This will mutate the provided models to include db managed column values. @@ -518,7 +541,7 @@ async def delete_records(cls, **kwargs: Any) -> None: raise InvalidFieldValue(message=e.diag.message_primary) @classmethod - async def update_record(cls, **kwargs: Any) -> "DatabaseModel": + async def update_record(cls: Type[SelfType], **kwargs: Any) -> SelfType: """Update a record associated with this DatabaseModel Notes: @@ -550,8 +573,10 @@ async def update_record(cls, **kwargs: Any) -> "DatabaseModel": @classmethod async def update( - cls, where_expressions: Optional[List[BinaryExpression]], values: Dict[str, Any] - ) -> List["DatabaseModel"]: + cls: Type[SelfType], + where_expressions: Optional[List[BinaryExpression]], + values: Dict[str, Any], + ) -> List[SelfType]: """Execute an UPDATE on a DatabaseModel table with the given parameters Args: @@ -584,7 +609,7 @@ async def update( return [cls.from_dict(dict(record)) for record in await results.fetchall()] - async def save(self, include_nested_models=False) -> None: + async def save(self, include_nested_models: bool = False) -> None: """Update the database record this object represents with its current state Args: @@ -685,7 +710,7 @@ async def delete(self) -> None: raise InvalidFieldValue(message=e.diag.message_primary) @classmethod - def from_dict(cls, _dict: Dict[str, Any]) -> "DatabaseModel": + def from_dict(cls: Type[SelfType], _dict: Dict[str, Any]) -> SelfType: """Instantiate a DatabaseModel object from a dict record Note: diff --git a/pynocular/exceptions.py b/pynocular/exceptions.py index 7a707ea..82bc64b 100644 --- a/pynocular/exceptions.py +++ b/pynocular/exceptions.py @@ -1,4 +1,5 @@ """Exceptions module for ORM package""" +from enum import Enum import traceback from typing import Any, Dict, Iterable, Optional @@ -89,7 +90,7 @@ class BaseException(Exception): # This property denotes the component that owns will raise this exception. This # property can be used for component level monitoring - COMPONENT = None + COMPONENT: Optional[Enum] = None def __init__( self, @@ -149,7 +150,9 @@ def get_metadata(self) -> Dict[str, str]: A dictionary of property keys to their stringified values """ - return {"component": self.COMPONENT.name} + if self.COMPONENT is not None: + return {"component": self.COMPONENT.name} + return {} def __str__(self) -> str: """Returns the message describing the exception""" @@ -311,7 +314,7 @@ def __str__(self) -> str: class NestedDatabaseModelNotResolved(BaseException): """Indicates a property was accessed before the reference was resolved""" - def __init__(self, model_cls: str, nested_model_id_value: Any) -> None: + def __init__(self, model_cls: Any, nested_model_id_value: Any) -> None: """Initialize NestedDatabaseModelNotResolved Args: diff --git a/pynocular/nested_database_model.py b/pynocular/nested_database_model.py index a8f0c63..0ea3091 100644 --- a/pynocular/nested_database_model.py +++ b/pynocular/nested_database_model.py @@ -1,15 +1,18 @@ """Class that wraps nested DatabaseModels""" -from typing import Any, Callable +from typing import Any, Type, TYPE_CHECKING from pynocular.exceptions import NestedDatabaseModelNotResolved +if TYPE_CHECKING: + from pynocular.database_model import DatabaseModel + class NestedDatabaseModel: """Class that wraps nested DatabaseModels""" def __init__( self, - model_cls: Callable, + model_cls: Type["DatabaseModel"], _id: Any, model: "DatabaseModel" = None, # noqa ) -> None: diff --git a/pynocular/patch_models.py b/pynocular/patch_models.py index e9607ed..c473cf4 100644 --- a/pynocular/patch_models.py +++ b/pynocular/patch_models.py @@ -1,7 +1,7 @@ """Context manager for mocking db calls for DatabaseModels during tests""" from contextlib import contextmanager import functools -from typing import Any, Dict, List, Optional +from typing import Any, Dict, Generator, List, Optional, Type, TypeVar from unittest.mock import patch from uuid import uuid4 @@ -23,12 +23,14 @@ from pynocular.database_model import DatabaseModel +SelfType = TypeVar("SelfType", bound=DatabaseModel) + @contextmanager def patch_database_model( - model_cls: DatabaseModel, - models: Optional[List[DatabaseModel]] = None, -) -> None: + model_cls: Type[SelfType], + models: Optional[List[SelfType]] = None, +) -> Generator: """Patch a DatabaseModel class, seeding with a set of values Example: @@ -61,7 +63,7 @@ async def select( where_expressions: Optional[List[BinaryExpression]] = None, order_by: Optional[List[UnaryExpression]] = None, limit: Optional[int] = None, - ) -> List[DatabaseModel]: + ) -> List[SelfType]: """Mock select function for DatabaseModel Args: @@ -92,7 +94,7 @@ async def select( return matched_models - async def create_list(models) -> List[DatabaseModel]: + async def create_list(models: List[SelfType]) -> List[SelfType]: """Mock `create_list` function for DatabaseModel Args: @@ -108,7 +110,7 @@ async def create_list(models) -> List[DatabaseModel]: return models - async def save(model, include_nested_models=False) -> None: + async def save(model: SelfType, include_nested_models: bool = False) -> None: """Mock `save` function for DatabaseModel Args: @@ -158,7 +160,7 @@ async def save(model, include_nested_models=False) -> None: for attr, val in model.dict().items(): setattr(matched_model, attr, val) - async def update_record(**kwargs: Any) -> DatabaseModel: + async def update_record(**kwargs: Any) -> SelfType: """Mock `update_record` function for DatabaseModel Args: @@ -194,7 +196,7 @@ async def update_record(**kwargs: Any) -> DatabaseModel: async def update( where_expressions: Optional[List[BinaryExpression]], values: Dict[str, Any] - ) -> List[DatabaseModel]: + ) -> List[SelfType]: """Mock `update_record` function for DatabaseModel Args: @@ -213,7 +215,7 @@ async def update( setattr(model, attr, val) return models - async def delete(model) -> None: + async def delete(model: SelfType) -> None: """Mock `delete` function for DatabaseModel""" primary_keys = model_cls._primary_keys diff --git a/setup.cfg b/setup.cfg index ed5e314..ea0a12e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,6 +31,7 @@ follow_imports=silent disallow_untyped_defs=True disallow_incomplete_defs=True warn_unreachable=True +plugins = pydantic.mypy [codespell] check-filenames= From f39f789dd45f930742c50cd8fb3c57488bec6393 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 13:49:59 -0500 Subject: [PATCH 02/10] Add readme --- README.md | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/README.md b/README.md index d0a9fc6..59f5a04 100644 --- a/README.md +++ b/README.md @@ -450,6 +450,60 @@ with patch_database_model(Org, models=orgs), patch_database_model( assert org_get.tech_owner.username == "bberkley" ``` +### Type checking and mypy support + +Pynocular supports mypy in a limited way. Using mypy with the Pydantic mypy +plugin will provide type checking for Pynocular database models' usual pydantic +fields, but by default mypy will not know about the DatabaseModel specific +methods. + +e.g. +```python +from pynocular.database_model import database_model + +@database_model(...) +class MyModel(BaseModel): + my_field: str = ... + +model = MyModel(...) # mypy understands this type +print(model.my_field) # mypy will not error on this +print(model.non_existent_field) # mypy will error here +model_list = await MyModel.get_list(...) # mypy will (incorrectly) error here +``` + +The false positives are due to a limitation of python's type hint system that +doesn't allow for dynamic class modification. The current workaround is to +explicitly construct a new type subclassing your model and data model to +provide to the type checker. + +e.g. + +```python +from pynocular.database_model import database_model, DatabaseModel +from typing import TYPE_CHECKING + +@database_model(...) +class MyModel_(BaseModel): + my_field: str = ... + +# This code is only used for the type checker +if TYPE_CHECKING: + class MyModel(DatabaseModel): + pass +else: + MyModel = MyModel_ + +model = MyModel(...) # mypy understands this type +print(model.my_field) # mypy will not error on this +print(model.non_existent_field) # mypy will error here +model_list = await MyModel.get_list(...) # mypy will not error anymore +print(model_list[0].non_existent_field) # mypy knows the type of model_list is + # List[MyModel], so catches the missing attribute +``` + +In the future, this functionality may be rolled into a mypy plugin to avoid +requiring the `TYPE_CHECKING` block. + ## Development To develop Pynocular, install dependencies and enable the pre-commit hook. From 0f3aad18e9fdee4181f9b7fa2d75bd3f78233d81 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 15:01:35 -0500 Subject: [PATCH 03/10] Gate changes behind tpye checking --- pynocular/database_model.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pynocular/database_model.py b/pynocular/database_model.py index f0cd173..a530a82 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -14,6 +14,7 @@ Sequence, Set, Type, + TYPE_CHECKING, TypeVar, Union, ) @@ -162,8 +163,13 @@ def wrapped(cls: T) -> T: SelfType = TypeVar("SelfType", bound="DatabaseModel") +if TYPE_CHECKING: + _pydantic_base_model = BaseModel +else: + _pydantic_base_model = object -class DatabaseModel(BaseModel): + +class DatabaseModel(_pydantic_base_model): """Adds database functionality to a Pydantic BaseModel A DatabaseModel is a Pydantic based model along with a SQLAlchemy From 90d65a3681adc8099fa9f39663f9ae92edd8bd95 Mon Sep 17 00:00:00 2001 From: ns-circle-ci Date: Mon, 14 Mar 2022 20:03:13 +0000 Subject: [PATCH 04/10] Version bumped to 0.19.0 --- pynocular/__init__.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pynocular/__init__.py b/pynocular/__init__.py index b9a83dc..2429f99 100644 --- a/pynocular/__init__.py +++ b/pynocular/__init__.py @@ -1,6 +1,6 @@ """Lightweight ORM that lets you query your database using Pydantic models and asyncio""" -__version__ = "0.18.0" +__version__ = "0.19.0" from pynocular.database_model import DatabaseModel, UUID_STR from pynocular.engines import DatabaseType, DBInfo diff --git a/pyproject.toml b/pyproject.toml index adcad6d..2a251d9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "pynocular" -version = "0.18.0" +version = "0.19.0" description = "Lightweight ORM that lets you query your database using Pydantic models and asyncio" authors = [ "RJ Santana ", From 61b58fe4bc35b2eb9bc023b0f4e56854300464fd Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 15:41:39 -0500 Subject: [PATCH 05/10] Better workarounds for library users --- README.md | 50 ++++++++++++++++++++++++++----------- pynocular/database_model.py | 28 ++++++++++++++++----- 2 files changed, 58 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 59f5a04..5b2f1f1 100644 --- a/README.md +++ b/README.md @@ -460,6 +460,7 @@ methods. e.g. ```python from pynocular.database_model import database_model +from pydantic import BaseModel @database_model(...) class MyModel(BaseModel): @@ -472,37 +473,58 @@ model_list = await MyModel.get_list(...) # mypy will (incorrectly) error here ``` The false positives are due to a limitation of python's type hint system that -doesn't allow for dynamic class modification. The current workaround is to -explicitly construct a new type subclassing your model and data model to -provide to the type checker. +doesn't allow for dynamic class modification. The current workaround is to use +the `BaseModel_database_model_hint` class provided by Pynocular, which is +identical at runtime to the Pydantic one, but with extra type information. For +convenience, the unmodified `BaseModel` class is also available to be imported +from the same place. + +Warning: if you use `BaseModel_database_model_hint` without the +`database_model` decorator, mypy will incorrectly believe the model will have +the pynocular provided functionality. e.g. ```python -from pynocular.database_model import database_model, DatabaseModel -from typing import TYPE_CHECKING +from pynocular.database_model import database_model, BaseModel, BaseModel_database_model_hint @database_model(...) -class MyModel_(BaseModel): +class MyModel_(BaseModel_database_model_hint): my_field: str = ... -# This code is only used for the type checker -if TYPE_CHECKING: - class MyModel(DatabaseModel): - pass -else: - MyModel = MyModel_ - model = MyModel(...) # mypy understands this type print(model.my_field) # mypy will not error on this print(model.non_existent_field) # mypy will error here model_list = await MyModel.get_list(...) # mypy will not error anymore print(model_list[0].non_existent_field) # mypy knows the type of model_list is # List[MyModel], so catches the missing attribute +model_list[0].my_field = "foo" # mypy doesn't error +await model_list[0].save(some_bad_arg) # mypy errors on the incorrect call to save + +class MyNonDbModel(BaseModel): + my_other_field: str = ... + +other_list = await MyNonDbModel.create_list() # Mypy correctly errors on + # attempted use of pynocular provided method + ``` In the future, this functionality may be rolled into a mypy plugin to avoid -requiring the `TYPE_CHECKING` block. +requiring the separate `BaseModel` imports. + +#### Nested Models + +Currently, Pynocular doesn't support nested models with mypy. As a workaround, +you can append `# type: ignore` to lines using `nested_model` as a type +annotation: + +```python + +class MyModel(BaseModel): + nested_db_field: nested_model(...) = ... # type: ignore + +``` + ## Development diff --git a/pynocular/database_model.py b/pynocular/database_model.py index a530a82..946b99a 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -93,9 +93,12 @@ def validate(cls, v: Any) -> str: raise ValueError("invalid UUID string") +SelfType = TypeVar("SelfType", bound="DatabaseModel") + + def nested_model( - db_model_class: Type["DatabaseModel"], reference_field: str = None -) -> Callable: + db_model_class: Type[SelfType], reference_field: str = None +) -> Type[SelfType]: """Generate a NestedModel class with dynamic model references Args: @@ -127,7 +130,7 @@ def validate(cls, v: Union[UUID_STR, "DatabaseModel"]) -> NestedDatabaseModel: v = cast("DatabaseModel", v) return NestedDatabaseModel(db_model_class, v.get_primary_id(), v) - return NestedModel + return NestedModel # type: ignore T = TypeVar("T", bound=Type[BaseModel]) @@ -161,8 +164,6 @@ def wrapped(cls: T) -> T: return wrapped -SelfType = TypeVar("SelfType", bound="DatabaseModel") - if TYPE_CHECKING: _pydantic_base_model = BaseModel else: @@ -448,7 +449,7 @@ async def select( return [cls.from_dict(dict(record)) for record in records] @classmethod - async def create(cls: Type[SelfType], **data: Dict[str, Any]) -> SelfType: + async def create(cls: Type[SelfType], **data: Any) -> SelfType: """Create a new instance of the this DatabaseModel and save it Args: @@ -779,3 +780,18 @@ def to_dict( _dict[prop_name] = prop_value return _dict + + +# Used for clients to import BaseModel from here and allow mypy to understand +# that both BaseModel and DatabaseModel functions are valid for subclasses with +# the database_model decorator applied +if TYPE_CHECKING: + + class BaseModel_database_model_hint(DatabaseModel, BaseModel): + """Dummy class to keep mypy happy""" + + pass + + +else: + BaseModel_database_model_hint = BaseModel From 9edaeea07e3a169d09ccc14ef63fd632f4f89658 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 15:43:36 -0500 Subject: [PATCH 06/10] Better readme --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 5b2f1f1..1178efc 100644 --- a/README.md +++ b/README.md @@ -452,10 +452,10 @@ with patch_database_model(Org, models=orgs), patch_database_model( ### Type checking and mypy support -Pynocular supports mypy in a limited way. Using mypy with the Pydantic mypy -plugin will provide type checking for Pynocular database models' usual pydantic -fields, but by default mypy will not know about the DatabaseModel specific -methods. +Pynocular is fully type hinted and supports mypy in a limited way. Using mypy +with the Pydantic mypy plugin will provide type checking for Pynocular database +models' usual pydantic fields, but by default mypy will not know about the +DatabaseModel specific methods. e.g. ```python From db800b3244c68ce86a3ddebc4fb33938670d49c0 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Mon, 14 Mar 2022 15:54:49 -0500 Subject: [PATCH 07/10] More readme updates --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1178efc..7965a3b 100644 --- a/README.md +++ b/README.md @@ -479,9 +479,9 @@ identical at runtime to the Pydantic one, but with extra type information. For convenience, the unmodified `BaseModel` class is also available to be imported from the same place. -Warning: if you use `BaseModel_database_model_hint` without the -`database_model` decorator, mypy will incorrectly believe the model will have -the pynocular provided functionality. +Warning: if you use `BaseModel_database_model_hint` to create a Pydantic model +without the `database_model` decorator, mypy will incorrectly believe the model +will have the pynocular provided functionality. e.g. From 87205037bd52323ba51a2f490683d73894ad22d2 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Sat, 19 Mar 2022 14:45:37 -0500 Subject: [PATCH 08/10] Prototype of new functionality --- pynocular/database_model.py | 72 ++++++++++++++++++++++++++------- tests/unit/test_patch_models.py | 10 +++-- 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/pynocular/database_model.py b/pynocular/database_model.py index 946b99a..661f53c 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -1,5 +1,6 @@ """Base Model class that implements CRUD methods for database entities based on Pydantic dataclasses""" import asyncio +import copy from datetime import datetime from enum import Enum, EnumMeta import inspect @@ -22,6 +23,7 @@ from aenum import Enum as AEnum, EnumMeta as AEnumMeta from pydantic import BaseModel, PositiveFloat, PositiveInt +from pydantic.main import ModelMetaclass from pydantic.types import UUID4 from sqlalchemy import ( and_, @@ -93,12 +95,19 @@ def validate(cls, v: Any) -> str: raise ValueError("invalid UUID string") -SelfType = TypeVar("SelfType", bound="DatabaseModel") +# DatabaseModel methods often return objects of their own type +# i.e. await SomeModel.get_list(...) returns type list[SomeModel], not +# list[DatabaseModel], so we use a generic to capture that +SelfType = TypeVar("SelfType", bound="_DatabaseModel") + + +# nested_model has a similar thing, but uses the public-facing DatabaseModel +PublicSelfType = TypeVar("PublicSelfType", bound="DatabaseModel") def nested_model( - db_model_class: Type[SelfType], reference_field: str = None -) -> Type[SelfType]: + db_model_class: Type[PublicSelfType], reference_field: str = None +) -> Type[PublicSelfType]: """Generate a NestedModel class with dynamic model references Args: @@ -156,7 +165,7 @@ def wrapped(cls: T) -> T: "Model is not subclass of pydantic.BaseModel" ) - cls.__bases__ += (DatabaseModel,) + cls.__bases__ += (_DatabaseModel,) cls.initialize_table(table_name, database_info) # type: ignore return cls @@ -164,13 +173,19 @@ def wrapped(cls: T) -> T: return wrapped +# Tell mypy that _DatabaseModle subclasses BaseModel so it knows that using +# BaseModel's attributes is acceptable in _DatabaseModel methods +# in practice, all subclasses will subclass BaseModel if TYPE_CHECKING: _pydantic_base_model = BaseModel else: + # At runtime, we can't subclass BaseModel directly because the metaclass + # would treat all DatabaseModel attributes as pydantic fields, for e.g. + # validation _pydantic_base_model = object -class DatabaseModel(_pydantic_base_model): +class _DatabaseModel(_pydantic_base_model): """Adds database functionality to a Pydantic BaseModel A DatabaseModel is a Pydantic based model along with a SQLAlchemy @@ -209,6 +224,23 @@ class DatabaseModel(_pydantic_base_model): # This can be used to access the table when defining where expressions columns: ImmutableColumnCollection = None + def __init_subclass__( + cls, table_name: str = None, database_info: DBInfo = None, **kwargs: Any + ) -> None: + """When a new DB model is created, initialize the table + + Args: + table_name: The name of the table associated with this model + database_info: The database information associated with this model + + """ + super().__init_subclass__(**kwargs) + if _DatabaseModel not in inspect.getmro(cls): + # Assume we're using the class decorator if we subclass BaseModel + return + + cls.initialize_table(table_name, database_info) + @classmethod def initialize_table(cls, table_name: str, database_info: DBInfo) -> None: """Returns a SQLAlchemy table definition to expose SQLAlchemy functions @@ -309,7 +341,7 @@ def initialize_table(cls, table_name: str, database_info: DBInfo) -> None: cls.columns = cls._table.c @classmethod - async def get_with_refs(cls, *args: Any, **kwargs: Any) -> "DatabaseModel": + async def get_with_refs(cls, *args: Any, **kwargs: Any) -> "_DatabaseModel": """Gets the DatabaseModel associated with any nested key references resolved Args: @@ -782,16 +814,26 @@ def to_dict( return _dict -# Used for clients to import BaseModel from here and allow mypy to understand -# that both BaseModel and DatabaseModel functions are valid for subclasses with -# the database_model decorator applied -if TYPE_CHECKING: +# Create public class to use to create type checkable DatabaseModels +class DatabaseModel(_DatabaseModel, BaseModel): + pass - class BaseModel_database_model_hint(DatabaseModel, BaseModel): - """Dummy class to keep mypy happy""" - pass +class MyModel( + DatabaseModel, table_name="my_table", database_info=DBInfo("type", (("a", "b"),)) +): + field: str = "" + other: int = 0 +class NestedModel(DatabaseModel, table_name = "table2", database_info=DBInfo("type")): + nest: nested_model(MyModel, reference_field="nest") # type: ignore -else: - BaseModel_database_model_hint = BaseModel +async def model() -> None: + m_list = await MyModel.get_list() + m = m_list[0] + await m.save() + print(m.field) + print(m.bad_field) # Mypy error! + nest_list = await NestedModel.get_list() + n = nest_list[0] + reveal_type(n.nest) # Any, since we used type: ignore for definition diff --git a/tests/unit/test_patch_models.py b/tests/unit/test_patch_models.py index 55bddc7..632057e 100644 --- a/tests/unit/test_patch_models.py +++ b/tests/unit/test_patch_models.py @@ -6,7 +6,12 @@ import pytest from sqlalchemy import or_ -from pynocular.database_model import database_model, nested_model, UUID_STR +from pynocular.database_model import ( + database_model, + DatabaseModel, + nested_model, + UUID_STR, +) from pynocular.engines import DBInfo from pynocular.patch_models import _evaluate_column_element, patch_database_model @@ -16,8 +21,7 @@ name = "boo" -@database_model("users", testdb) -class User(BaseModel): +class User(DatabaseModel, table_name="users", database_info=testdb): """Model that represents the `users` table""" id: UUID_STR = Field(primary_key=True) From 3c00a5696428de23779ad66d892eba6d309e2ff2 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Sat, 19 Mar 2022 17:05:57 -0500 Subject: [PATCH 09/10] Add nested model workaround --- pynocular/database_model.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pynocular/database_model.py b/pynocular/database_model.py index 661f53c..4d2845c 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -826,7 +826,10 @@ class MyModel( other: int = 0 class NestedModel(DatabaseModel, table_name = "table2", database_info=DBInfo("type")): - nest: nested_model(MyModel, reference_field="nest") # type: ignore + if TYPE_CHECKING: + nest: MyModel + else: + nest: nested_model(MyModel, reference_field="nest") async def model() -> None: m_list = await MyModel.get_list() @@ -836,4 +839,4 @@ async def model() -> None: print(m.bad_field) # Mypy error! nest_list = await NestedModel.get_list() n = nest_list[0] - reveal_type(n.nest) # Any, since we used type: ignore for definition + reveal_type(n.nest) # MyModel, since we used TYPE_CHECKING in definition From 4cdc45f6f245515a37097a28282727e33f1e79c9 Mon Sep 17 00:00:00 2001 From: Brendan Gimby Date: Sat, 19 Mar 2022 17:10:58 -0500 Subject: [PATCH 10/10] More POC code --- pynocular/database_model.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pynocular/database_model.py b/pynocular/database_model.py index 4d2845c..e800080 100644 --- a/pynocular/database_model.py +++ b/pynocular/database_model.py @@ -819,6 +819,8 @@ class DatabaseModel(_DatabaseModel, BaseModel): pass +#-------------- Prototype code to mess around with ---------------- + class MyModel( DatabaseModel, table_name="my_table", database_info=DBInfo("type", (("a", "b"),)) ): @@ -832,11 +834,14 @@ class NestedModel(DatabaseModel, table_name = "table2", database_info=DBInfo("ty nest: nested_model(MyModel, reference_field="nest") async def model() -> None: - m_list = await MyModel.get_list() - m = m_list[0] + m_list = MyModel.get_list() + m = m_list[0] # Mypy error: we didn't await get_list + m = (await m_list)[0] await m.save() print(m.field) - print(m.bad_field) # Mypy error! + print(m.bad_field) # Mypy error, bad_field isn't an attribute of MyModel nest_list = await NestedModel.get_list() n = nest_list[0] reveal_type(n.nest) # MyModel, since we used TYPE_CHECKING in definition + n.nest.field = "abc" + n.nest.field = 1 # Mypy error: MyModel.field is a string, not int