Fix miscellaneous type annotation issues - #371

This can also be a class derived from the declarative base. I'd love to add typing everywhere, is there a reason this parameter in particular gave you a problem?

I dug into how ‎user_cls is actually used and wanted to check what level of strictness you’re comfortable with for the type here.

Right now ‎make_versioned just passes ‎user_cls through to the ‎VersioningManager, and later it’s used like this:

if manager.user_cls:
    user_cls = manager.user_cls
    Base = manager.declarative_base
    registry = Base.registry._class_registry

    if isinstance(user_cls, str):
        try:
            user_cls = registry[user_cls]
        except KeyError:
            raise ImproperlyConfigured(
                'Could not build relationship between Transaction'
                f' and {user_cls}. {user_cls} was not found in declarative class '
                'registry. Either configure VersioningManager to '
                'use different user class or disable this '
                'relationship '
            )

    user_id = sa.Column(
        sa.inspect(user_cls).primary_key[0].type,
        sa.ForeignKey(sa.inspect(user_cls).primary_key[0]),
        index=True,
    )

    user = sa.orm.relationship(user_cls)

So effectively, when it’s not ‎None, ‎user_cls is:

• either a string key into ‎Base.registry._class_registry, or

• a mapped class that ‎sa.inspect(user_cls) can handle and that has a primary key.

Given that, the “ideal” type annotation would be something like “class from this declarative registry or a string name of such a class”, but Python’s type system can’t express “class from this particular registry”.

Because of that, I see three options:

1. If we’re willing to support SQLAlchemy 2.0+ only and the modern declarative base, we could tighten this to:

from sqlalchemy.orm import DeclarativeBase

UserCls = type[DeclarativeBase] | str | None

def make_versioned(
    ...,
    user_cls: UserCls = "User",
) -> None:
    ...

That would be the most precise for new-style declarative users, but it would be wrong for older ‎declarative_base() and classical mappings.

2. If we need to keep compatibility with older declarative bases / classical mappings, then the most honest library-level type is the looser:

from typing import Any

UserCls = type[Any] | str | None

def make_versioned(
    ...,
    user_cls: UserCls = "User",
) -> None:
    ...

This matches what the code actually does (any mapped class that ‎inspect() can handle, plus string name, plus ‎None) without incorrectly excluding valid usages.

3. If neither of these is acceptable (i.e. we don’t want to tighten the supported SQLAlchemy surface area and also don’t want the looser ‎type[Any] | str | None), then I’d agree we leave the parameter unannotated and close this PR, since anything stricter would be lying about the real constraints.

Let me know which direction you’d prefer and I can adjust the annotation (or drop it) accordingly.