Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static typing using Cast expects two type arguments #115

Open
mdczaplicki opened this issue May 28, 2021 · 11 comments
Open

Static typing using Cast expects two type arguments #115

mdczaplicki opened this issue May 28, 2021 · 11 comments
Labels
bug Something isn't working use case

Comments

@mdczaplicki
Copy link

Describe the bug
We have a statically typed dictionary that has values of type Cast[Decimal] & it seems to be valid, since we cast column value to FLOAT. But the type annotation expects two type arguments. I think it uses this stub: https://github.com/sqlalchemy/sqlalchemy2-stubs/blob/master/sqlalchemy-stubs/sql/elements.pyi#L333
whereas it's for a method cast on ColumnElement.

Expected behavior
Static typing using Cast[Decimal] works fine :P

To Reproduce

from uuid import uuid4, UUID
from typing import Dict
from sqlalchemy.sql.elements import Cast
from decimal import Decimal
from sqlalchemy import FLOAT, cast


class X:
  y: Optional[float]
x =x()
x.y = 2.0

payload: Dict[UUID, Cast[Decimal]] = {
    uuid4(): cast(x.y, FLOAT)
}

Error

error: "Cast" expects 2 type arguments, but 1 given

Versions.

  • OS: Linux Mint 19.3 Cinnamon
  • Python: 3.7.10
  • SQLAlchemy: 1.4.15
  • Database: PostgreSQL
  • DBAPI: asyncpg

Additional context
Maybe we should actually not use cast in this case? We are using it because y is an Optional[float] & not float, but we are 100% sure that it's not None?

Have a nice day!

@mdczaplicki mdczaplicki added the requires triage New issue that requires categorization label May 28, 2021
@mdczaplicki
Copy link
Author

@MaicoTimmerman, bump 😛

@zzzeek
Copy link
Member

zzzeek commented Jun 24, 2021

SQLAlchemy's Cast accepts a TypeEngine object, not a Python type. So here the more correct thing would be cast[sqlalchemy.types.Numeric]. We don't express SQL constructs in terms of Python datatypes, there is a layer of indirection from the TypeEngine.

@mdczaplicki
Copy link
Author

But this still would be one type argument, not two; isn't that right?

@zzzeek
Copy link
Member

zzzeek commented Jun 24, 2021

from my POV it seems like you care about the resulting type of the construct, not the originating type, so yes. but i may not be seeing the whole picture.

@mdczaplicki
Copy link
Author

Yeah, that makes sense that I've approached from the different direction. But nevertheless the issue still stands as present. Unless I need to provide both originating & resulting types. I'll check tomorrow.

@MaicoTimmerman
Copy link
Contributor

I've looked into the originating+resulting vs resulting only generic type. I believe to be fully typed, we need both. Cast exposes both the clause and wrapped_column_expression properties, which should be typed based on the source type, requiring 2 generic arguments.

However we'd need to do some more thinking about the source type to be ORM compatible. Currently, it expects a source to be inherited from ClauseElement, however ORM mapped instance attributes are python types:

from sqlalchemy import cast, Float, Column, Numeric
from sqlalchemy.orm import declarative_base

Base = declarative_base()


class X(Base):
    y = Column(Float)


x = X()
x.y = 2.0
reveal_type(x.y)
reveal_type(cast(x.y, Numeric))  # error: Value of type variable "_CLE" of "Cast" cannot be "float"
reveal_type(cast(X.y, Numeric))  # works

I also noted that the generic is currently Cast[<result_type>, <source_type>], which I'd find to be counter-intuitive, as the generic order is reversed from the calling argument order.

@MaicoTimmerman MaicoTimmerman added bug Something isn't working use case and removed requires triage New issue that requires categorization labels Jun 25, 2021
@zzzeek
Copy link
Member

zzzeek commented Jun 25, 2021

I've looked into the originating+resulting vs resulting only generic type. I believe to be fully typed, we need both. Cast exposes both the clause and wrapped_column_expression properties, which should be typed based on the source type, requiring 2 generic arguments.

Well the ".clause" of the Cast is really not important on the outside, it's there for informational purposes but does not determine any meaningful behavior of the construct, only the specifics of the string that is generated in compilation. There has to be some kind of boundary for what we put in the [] of a generic type. If I have an object that has ".x = int, .y = dict, .foo = Bar()" as members, we hardly would normally be passing around things like MyObject[int, dict, Bar] - there's some threshold of what's actually important to be in the generic slot, and in this case 100% of the behavior of the object as manipulated in Python and used in idiomatic patterns is determined by the target type, not the source type.

I also noted that the generic is currently Cast[<result_type>, <source_type>], which I'd find to be counter-intuitive, as the generic order is reversed from the calling argument order.

I don't think every member of an object that happens to have a SQL type needs to be part of the generic representation, and perhaps we should try to clarify what really goes into the [] and why.

@anentropic
Copy link

anentropic commented Oct 4, 2022

based on this issue I had some code like:

    rows = await session.execute(
        sql.select([func.pg_try_advisory_xact_lock(cast(key, BigInteger))])
    )

where key is a python integer, i.e. just a literal value for SQL and not a field expression

this worked fine in my unit tests, but mypy complains:

error: Value of type variable "_CLE" of "Cast" cannot be "int"

After a bit of experimenting I found this formulation that works and also satisfies mypy:

    rows = await session.execute(
        sql.select([func.pg_try_advisory_xact_lock(literal(key, BigInteger))])
    )

@peterschutt
Copy link
Contributor

cast() accepts the type as the first argument and the value as second arg, eg, cast(BigInteger, key).

@anentropic
Copy link

cast() accepts the type as the first argument and the value as second arg, eg, cast(BigInteger, key).

I think that's mypy cast ... here I am talking about from sqlachemy import cast

@peterschutt
Copy link
Contributor

Oh my bad! Totally read it as typing cast

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working use case
Projects
None yet
Development

No branches or pull requests

5 participants