Fix: geoalchemy2.functions type stubs

[mbway]

Description

I made a mistake when writing the function stub generator. I should have familiarised myself with geoalchemy2 first (I have never used it before, I contributed the types to improve type checking coverage in a codebase that I work with).

The dynamic 'functions' in the geoalchemy2.functions module are only markers to be used to construct SQL queries and do no computation themselves and thus the type hints were incorrect. I knew they served the SQL construction purpose but for some reason I was under the impression that they could also be used as regular functions eg

geom: WKBElement = ST_GeomFromText('POINT(0, 0)')
geom2: WKBElement = ST_Buffer(geom, 1)

but this is not the case.

I took a look at how functions are type hinted in SQLAlchemy, for example the SQLAlchemy documentation explains how to create a custom generic function:

class as_utc(GenericFunction):
    type = DateTime()
    inherit_cache = True

So the geoalchemy functions should look like this.

There is a more specific way these can be typed, quoting the latest SQLAlchemy source code:

Type parameters for [GenericFunction] as a
`generic type <https://peps.python.org/pep-0484/#generics>`_ can be passed
and should match the type seen in a :class:`_engine.Result`. For example::

        class as_utc(GenericFunction[datetime.datetime]):
            type = DateTime()
            inherit_cache = True

The above indicates that the following expression returns a ``datetime``
object::

        connection.scalar(select(func.as_utc()))

but this is not available in SQLAlchemy 1.4 which GeoAlchemy2 has as the minimum version and I also wasn't sure what to put as the value for GenericFunction[T]

Checklist

This pull request is:

• [ ] A documentation / typographical error fix
  • Good to go, no issue or tests are needed
• [X] A short code fix
  • [ ] Please include: Fixes: #<issue number> in the description if it solves an existing issue (which must include a complete example of the issue).
  • [ ] Please include tests that fail with the main branch and pass with the provided fix.
• [ ] A new feature implementation or update an existing feature
  • [ ] Please include: Fixes: #<issue number> in the description if it solves an existing issue (which must include a complete example of the feature).
  • [ ] Please include tests that cover every lines of the new/updated feature.
  • [ ] Please update the documentation to describe the new/updated feature.

[mbway]

I think this wasn't caught because the tests don't actually import or use anything from geoalchemy2.functions so there was nothing for the type checker to reject

[adrien-berchet]

I think this wasn't caught because the tests don't actually import or use anything from geoalchemy2.functions so there was nothing for the type checker to reject

That's possible indeed. Could you add a new test for this that fails with the main branch and passes with this PR plz?

[adrien-berchet]

Thanks for this quick fix anyway! :)

[mbway]

I have added a simple sanity check unit test that ensures that a simple query formed using geoalchemy2.functions is well typed. On the master branch, mypy catches the error with:

error: No overload variant of "select" matches argument type "Geometry"  [call-overload]

but on this branch everything is OK.

with the previous type stubs the return value of geoalchemy2.functions.ST_Buffer(geom, 1.0) was geoalchemy2.types.Geometry which is not what sqlalchemy.select is expecting.

[adrien-berchet]

Ok, I see, thanks! Let's merge this one, then I'll rebase #480 so you can test it for you own use cases before I release it.