pep-484: ORM public API, constructors

for the moment, abandoning using @overload with
relationship() and mapped_column().  The overloads
are very difficult to get working at all, and
the overloads that were there all wouldn't pass on
mypy.  various techniques of getting them to
"work", meaning having right hand side dictate
what's legal on the left, have mixed success
and wont give consistent results; additionally,
it's legal to have Optional / non-optional
independent of nullable in any case for columns.
relationship cases are less ambiguous but mypy
was not going along with things.

we have a comprehensive system of allowing
left side annotations to drive the right side,
in the absense of explicit settings on the right.
so type-centric SQLAlchemy will be left-side
driven just like dataclasses, and the various flags
and switches on the right side will just not be
needed very much.

in other matters, one surprise, forgot to remove string support
from orm.join(A, B, "somename") or do deprecations
for it in 1.4.   This is a really not-directly-used
structure barely
mentioned in the docs for many years, the example
shows a relationship being used, not a string, so
we will just change it to raise the usual error here.

Change-Id: Iefbbb8d34548b538023890ab8b7c9a5d9496ec6e

3 files changed, 87 insertions, 51 deletions

diff --git a/lib/sqlalchemy/ext/asyncio/scoping.py b/lib/sqlalchemy/ext/asyncio/scoping.py
index 33cf3f745..c7a6e2ca0 100644
--- a/lib/sqlalchemy/ext/asyncio/scoping.py
+++ b/lib/sqlalchemy/ext/asyncio/scoping.py
@@ -76,7 +76,6 @@ if TYPE_CHECKING:
         "expunge",
         "expunge_all",
         "flush",
-        "get",
         "get_bind",
         "is_modified",
         "invalidate",
@@ -204,6 +203,49 @@ class async_scoped_session:
             await self.registry().close()
         self.registry.clear()
 
+    async def get(
+        self,
+        entity: _EntityBindKey[_O],
+        ident: _PKIdentityArgument,
+        *,
+        options: Optional[Sequence[ORMOption]] = None,
+        populate_existing: bool = False,
+        with_for_update: Optional[ForUpdateArg] = None,
+        identity_token: Optional[Any] = None,
+        execution_options: _ExecuteOptionsParameter = util.EMPTY_DICT,
+    ) -> Optional[_O]:
+        r"""Return an instance based on the given primary key identifier,
+        or ``None`` if not found.
+
+        .. container:: class_bases
+
+            Proxied for the :class:`_asyncio.AsyncSession` class on
+            behalf of the :class:`_asyncio.scoping.async_scoped_session` class.
+
+        .. seealso::
+
+            :meth:`_orm.Session.get` - main documentation for get
+
+
+
+        """  # noqa: E501
+
+        # this was proxied but Mypy is requiring the return type to be
+        # clarified
+
+        # work around:
+        # https://github.com/python/typing/discussions/1143
+        return_value = await self._proxied.get(
+            entity,
+            ident,
+            options=options,
+            populate_existing=populate_existing,
+            with_for_update=with_for_update,
+            identity_token=identity_token,
+            execution_options=execution_options,
+        )
+        return return_value
+
     # START PROXY METHODS async_scoped_session
 
     # code within this block is **programmatically,
@@ -632,43 +674,6 @@ class async_scoped_session:
 
         return await self._proxied.flush(objects=objects)
 
-    async def get(
-        self,
-        entity: _EntityBindKey[_O],
-        ident: _PKIdentityArgument,
-        *,
-        options: Optional[Sequence[ORMOption]] = None,
-        populate_existing: bool = False,
-        with_for_update: Optional[ForUpdateArg] = None,
-        identity_token: Optional[Any] = None,
-        execution_options: _ExecuteOptionsParameter = util.EMPTY_DICT,
-    ) -> Optional[_O]:
-        r"""Return an instance based on the given primary key identifier,
-        or ``None`` if not found.
-
-        .. container:: class_bases
-
-            Proxied for the :class:`_asyncio.AsyncSession` class on
-            behalf of the :class:`_asyncio.scoping.async_scoped_session` class.
-
-        .. seealso::
-
-            :meth:`_orm.Session.get` - main documentation for get
-
-
-
-        """  # noqa: E501
-
-        return await self._proxied.get(
-            entity,
-            ident,
-            options=options,
-            populate_existing=populate_existing,
-            with_for_update=with_for_update,
-            identity_token=identity_token,
-            execution_options=execution_options,
-        )
-
     def get_bind(
         self,
         mapper: Optional[_EntityBindKey[_O]] = None,
diff --git a/lib/sqlalchemy/ext/hybrid.py b/lib/sqlalchemy/ext/hybrid.py
index be872804e..7200414a1 100644
--- a/lib/sqlalchemy/ext/hybrid.py
+++ b/lib/sqlalchemy/ext/hybrid.py
@@ -832,6 +832,8 @@ from ..util.typing import Protocol
 
 
 if TYPE_CHECKING:
+    from ..orm._typing import _ORMColumnExprArgument
+    from ..orm.interfaces import MapperProperty
     from ..orm.util import AliasedInsp
     from ..sql._typing import _ColumnExpressionArgument
     from ..sql._typing import _DMLColumnArgument
@@ -840,7 +842,6 @@ if TYPE_CHECKING:
     from ..sql.operators import OperatorType
     from ..sql.roles import ColumnsClauseRole
 
-
 _T = TypeVar("_T", bound=Any)
 _T_co = TypeVar("_T_co", bound=Any, covariant=True)
 _T_con = TypeVar("_T_con", bound=Any, contravariant=True)
@@ -1289,7 +1290,7 @@ class Comparator(interfaces.PropComparator[_T]):
     ):
         self.expression = expression
 
-    def __clause_element__(self) -> ColumnsClauseRole:
+    def __clause_element__(self) -> _ORMColumnExprArgument[_T]:
         expr = self.expression
         if is_has_clause_element(expr):
             ret_expr = expr.__clause_element__()
@@ -1298,10 +1299,15 @@ class Comparator(interfaces.PropComparator[_T]):
                 assert isinstance(expr, ColumnElement)
             ret_expr = expr
 
+        if TYPE_CHECKING:
+            # see test_hybrid->test_expression_isnt_clause_element
+            # that exercises the usual place this is caught if not
+            # true
+            assert isinstance(ret_expr, ColumnElement)
         return ret_expr
 
-    @util.non_memoized_property
-    def property(self) -> Any:
+    @util.ro_non_memoized_property
+    def property(self) -> Optional[interfaces.MapperProperty[_T]]:
         return None
 
     def adapt_to_entity(
@@ -1325,7 +1331,7 @@ class ExprComparator(Comparator[_T]):
     def __getattr__(self, key: str) -> Any:
         return getattr(self.expression, key)
 
-    @util.non_memoized_property
+    @util.ro_non_memoized_property
     def info(self) -> _InfoType:
         return self.hybrid.info
 
@@ -1339,8 +1345,8 @@ class ExprComparator(Comparator[_T]):
         else:
             return [(self.expression, value)]
 
-    @util.non_memoized_property
-    def property(self) -> Any:
+    @util.ro_non_memoized_property
+    def property(self) -> Optional[MapperProperty[_T]]:
         return self.expression.property  # type: ignore
 
     def operate(
diff --git a/lib/sqlalchemy/ext/instrumentation.py b/lib/sqlalchemy/ext/instrumentation.py
index 72448fbdc..b1138a4ad 100644
--- a/lib/sqlalchemy/ext/instrumentation.py
+++ b/lib/sqlalchemy/ext/instrumentation.py
@@ -25,6 +25,7 @@ from ..orm import exc as orm_exc
 from ..orm import instrumentation as orm_instrumentation
 from ..orm.instrumentation import _default_dict_getter
 from ..orm.instrumentation import _default_manager_getter
+from ..orm.instrumentation import _default_opt_manager_getter
 from ..orm.instrumentation import _default_state_getter
 from ..orm.instrumentation import ClassManager
 from ..orm.instrumentation import InstrumentationFactory
@@ -140,7 +141,7 @@ class ExtendedInstrumentationRegistry(InstrumentationFactory):
         hierarchy = util.class_hierarchy(cls)
         factories = set()
         for member in hierarchy:
-            manager = self.manager_of_class(member)
+            manager = self.opt_manager_of_class(member)
             if manager is not None:
                 factories.add(manager.factory)
             else:
@@ -161,17 +162,34 @@ class ExtendedInstrumentationRegistry(InstrumentationFactory):
             del self._state_finders[class_]
             del self._dict_finders[class_]
 
-    def manager_of_class(self, cls):
-        if cls is None:
-            return None
+    def opt_manager_of_class(self, cls):
         try:
-            finder = self._manager_finders.get(cls, _default_manager_getter)
+            finder = self._manager_finders.get(
+                cls, _default_opt_manager_getter
+            )
         except TypeError:
             # due to weakref lookup on invalid object
             return None
         else:
             return finder(cls)
 
+    def manager_of_class(self, cls):
+        try:
+            finder = self._manager_finders.get(cls, _default_manager_getter)
+        except TypeError:
+            # due to weakref lookup on invalid object
+            raise orm_exc.UnmappedClassError(
+                cls, f"Can't locate an instrumentation manager for class {cls}"
+            )
+        else:
+            manager = finder(cls)
+            if manager is None:
+                raise orm_exc.UnmappedClassError(
+                    cls,
+                    f"Can't locate an instrumentation manager for class {cls}",
+                )
+            return manager
+
     def state_of(self, instance):
         if instance is None:
             raise AttributeError("None has no persistent state.")
@@ -384,6 +402,7 @@ def _install_instrumented_lookups():
             instance_state=_instrumentation_factory.state_of,
             instance_dict=_instrumentation_factory.dict_of,
             manager_of_class=_instrumentation_factory.manager_of_class,
+            opt_manager_of_class=_instrumentation_factory.opt_manager_of_class,
         )
     )
 
@@ -395,16 +414,19 @@ def _reinstall_default_lookups():
             instance_state=_default_state_getter,
             instance_dict=_default_dict_getter,
             manager_of_class=_default_manager_getter,
+            opt_manager_of_class=_default_opt_manager_getter,
         )
     )
     _instrumentation_factory._extended = False
 
 
 def _install_lookups(lookups):
-    global instance_state, instance_dict, manager_of_class
+    global instance_state, instance_dict
+    global manager_of_class, opt_manager_of_class
     instance_state = lookups["instance_state"]
     instance_dict = lookups["instance_dict"]
     manager_of_class = lookups["manager_of_class"]
+    opt_manager_of_class = lookups["opt_manager_of_class"]
     orm_base.instance_state = (
         attributes.instance_state
     ) = orm_instrumentation.instance_state = instance_state
@@ -414,3 +436,6 @@ def _install_lookups(lookups):
     orm_base.manager_of_class = (
         attributes.manager_of_class
     ) = orm_instrumentation.manager_of_class = manager_of_class
+    orm_base.opt_manager_of_class = (
+        attributes.opt_manager_of_class
+    ) = orm_instrumentation.opt_manager_of_class = opt_manager_of_class