Add type-checking support for SQLModel types.

[kdcokenny]

Added Type Checking Support for SQLModel Classes

Description

My changes solve #58 and allow users to directly use SQLModel without type-issues. It passes all tests and shows no typing issues.

Changes

I made many none-obtrusive type changes such as replacing instances of type[DeclarativeBase] with type[Union[DeclarativeBase, SQLModel]]. This alone caused mypy errors do to table not being present on SQLModel which doesn't make sense as it does, so I think this is just poor typing on SQLModel's side. To deal with this wherever model.title is being referenced I explicitly checked if that field was available and raised a ValueError if it was not. Doing this allowed my changes to pass all linting checks.

Checklist

• I have read the CONTRIBUTING document.
• My code follows the code style of this project.
• I have added necessary documentation (if appropriate).
• I have added tests that cover my changes (if applicable).
• All new and existing tests passed.

[igorbenav]

This will not work if sqlmodel is not installed. Since it's an optional dependency, it should check if sqlmodel is installed and also work if it's not installed

[kdcokenny]

I'll just leave this here for whoever wants to implement it then. Just needed this for my project.

[JakNowy]

Could we just add sqlmodel as regular dependency then? I think that's de facto the suggested library for fastcrud.

[kdcokenny]

Yeah I did a bit of research into this and the only way I see that we could truly allow it to be optional is to use the Any type and put from sqlmodel import SQLModel in a try except block. If this is an acceptable approach we could do it, but I cannot find any other workaround for this.

[kdcokenny]

So I did make the Any changes above. I also tried adding in run-time validation with:

...
    if not isinstance(model, DeclarativeBase):
        try:
            from sqlmodel.main import SQLModel, SQLModelMetaclass
            if not isinstance(model, Union[SQLModel, SQLModelMetaclass]):
                raise ValueError(
                    f"Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, "
                    f"but got {type(model).__name__}. "
                    f"Please ensure you're using a valid SQLAlchemy or SQLModel model."
                )
        except ImportError:
            raise ImportError(
                f"Invalid model type. Expected SQLAlchemy DeclarativeBase, but got {type(model).__name__}. "
                f"To use SQLModel, ensure you have installed the 'sqlmodel' package. "
                f"You can install it using 'pip install sqlmodel' or 'poetry add sqlmodel'."
            )

However, it kept failing tests with random model types which I could manually check for, but am not familiar enough with these libraries to confidently say that would not lead to massive unintended side-effects.

Note: ValueError: Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, but got DeclarativeAttributeIntercept. Please ensure you're using a valid SQLAlchemy or SQLModel model.

[kdcokenny]

Also, I would like to suggest adding Makefile to simplify testing, linting, and formatting. I find a universal make format, make lint, make test to be easier to remember than their counterparts like ruff check --fix. Also, isort for import sorting might be a nice addition, not my lib though so do whatever you prefer!

[JakNowy]

Also, I would like to suggest adding Makefile to simplify testing, linting, and formatting. I find a universal make format, make lint, make test to be easier to remember than their counterparts like ruff check --fix.

I am highly against this. Make introduces yet another layer of abstraction. We should stick to poetry and docker compose exec's whenever possible. What you might be interrested in is:

poetry run ruff check fastcrud

[igorbenav]

So I did make the Any changes above. I also tried adding in run-time validation with:

...
    if not isinstance(model, DeclarativeBase):
        try:
            from sqlmodel.main import SQLModel, SQLModelMetaclass
            if not isinstance(model, Union[SQLModel, SQLModelMetaclass]):
                raise ValueError(
                    f"Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, "
                    f"but got {type(model).__name__}. "
                    f"Please ensure you're using a valid SQLAlchemy or SQLModel model."
                )
        except ImportError:
            raise ImportError(
                f"Invalid model type. Expected SQLAlchemy DeclarativeBase, but got {type(model).__name__}. "
                f"To use SQLModel, ensure you have installed the 'sqlmodel' package. "
                f"You can install it using 'pip install sqlmodel' or 'poetry add sqlmodel'."
            )

However, it kept failing tests with random model types which I could manually check for, but am not familiar enough with these libraries to confidently say that would not lead to massive unintended side-effects.

Note: ValueError: Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, but got DeclarativeAttributeIntercept. Please ensure you're using a valid SQLAlchemy or SQLModel model.

Something like this would be good indeed. I think we could revisit in the future, but for now it looks good to me. Is it actually working with the example in the issue?

[igorbenav]

Also, isort for import sorting might be a nice addition, not my lib though so do whatever you prefer!

Ruff does this, ruff check --select I --fix. A github action to do this would be nice

[igorbenav]

Also, I would like to suggest adding Makefile to simplify testing, linting, and formatting. I find a universal make format, make lint, make test to be easier to remember than their counterparts like ruff check --fix.

I am highly against this. Make introduces yet another layer of abstraction. We should stick to poetry and docker compose exec's whenever possible. What you might be interrested in is:

poetry run ruff check fastcrud

I agree. The commands are in contributing.md, we could add them to readme as well to make it easier.

[kdcokenny]

So I did make the Any changes above. I also tried adding in run-time validation with:

...
    if not isinstance(model, DeclarativeBase):
        try:
            from sqlmodel.main import SQLModel, SQLModelMetaclass
            if not isinstance(model, Union[SQLModel, SQLModelMetaclass]):
                raise ValueError(
                    f"Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, "
                    f"but got {type(model).__name__}. "
                    f"Please ensure you're using a valid SQLAlchemy or SQLModel model."
                )
        except ImportError:
            raise ImportError(
                f"Invalid model type. Expected SQLAlchemy DeclarativeBase, but got {type(model).__name__}. "
                f"To use SQLModel, ensure you have installed the 'sqlmodel' package. "
                f"You can install it using 'pip install sqlmodel' or 'poetry add sqlmodel'."
            )

However, it kept failing tests with random model types which I could manually check for, but am not familiar enough with these libraries to confidently say that would not lead to massive unintended side-effects.
Note: ValueError: Invalid model type. Expected SQLAlchemy DeclarativeBase or SQLModel, but got DeclarativeAttributeIntercept. Please ensure you're using a valid SQLAlchemy or SQLModel model.

Something like this would be good indeed. I think we could revisit in the future, but for now it looks good to me. Is it actually working with the example in the issue?

Yes, the issues don't appear for me with my changes with his example code.