sqlalchemy2-stubs icon indicating copy to clipboard operation
sqlalchemy2-stubs copied to clipboard

stubs use implicit reexports

Open rectalogic opened this issue 4 years ago • 13 comments
trafficstars

Describe the bug mypy enables --no-implicit-reexport for stub files:

Note this is always treated as enabled for stub files.

https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-no-implicit-reexport Many of the sqlalchemy stubs reexport and so mypy seems to not recognize those exports.

Expected behavior mypy should not fail

To Reproduce cc.py:

import sqlalchemy

CC = sqlalchemy.sql.expression.ColumnClause

Error

$ mypy cc.py
cc.py:3: error: Module has no attribute "ColumnClause"
Found 1 error in 1 file (checked 1 source file)

Versions.

  • OS: Ubuntu 20.04
  • Python: 3.8.10
  • SQLAlchemy: 1.4.23
  • mypy: 0.910
  • SQLAlchemy2-stubs: 0.0.2a18

Additional context Manually editing .../lib/python3.8/site-packages/sqlalchemy-stubs/sql/expression.pyi and changing the import of ColumnClause to from .elements import ColumnClause as ColumnClause fixes this.

There are other objects that are also implicitly re-exported and result in mypy errors - sqlalchemy.sql.expression.Executable, sqlalchemy.sql.expression.FunctionElement, sqlalchemy.sql.expression.Tuple etc.

rectalogic avatar Oct 26 '21 19:10 rectalogic

Hi,

I'm not sure this is a bug actually. The elements that are not re-exported are not included in the the __all__ list in the original file: https://github.com/sqlalchemy/sqlalchemy/blob/main/lib/sqlalchemy/sql/expression.py

So I think we should decide if the public interface is __all__ or whatever it's imported in the module

CaselIT avatar Oct 26 '21 19:10 CaselIT

My usecase is I want to use these type definitions when typing my own code. e.g. I have some function that accepts a ColumnClause and so want to hint it:

def do_select(c: ColumnClause) -> Select:
    return select(c).select_from("user")

Unless all of these types are internal and not supposed to be used at all (in which case they should probably be renamed with a leading_ and not documented), then they should be exposed when I install the stubs.

Right now my code passes mypy unless I install the stubs, then I get dozens of failures due to all of these names in my codes hints no longer existing.

rectalogic avatar Oct 26 '21 20:10 rectalogic

I probably explained myself poorly.

The type is public in the sql.elements module. What is to decide is if it's public also in sql.expression. Once it is it cannot be removed from that module without a breaking change

CaselIT avatar Oct 26 '21 20:10 CaselIT

Hmm, I see. I have been using the documented type names - so sqlalchemy.sql.expression.ColumnClause instead of sqlalchemy.sql.elements.ColumnClause https://docs.sqlalchemy.org/en/14/core/sqlelement.html#sqlalchemy.sql.expression.ColumnClause

How would I even determine the correct non-reexported name to use in my own type hints other than tracing the imports thru the source code?

Anyway, changing my sample code above it now passes mypy

import sqlalchemy

CC = sqlalchemy.sql.elements.ColumnClause

rectalogic avatar Oct 26 '21 20:10 rectalogic

I tend to agree that probably anything that's in expression should be exported, since the module seems mostly a public interface one. @zzzeek thoughts?

CaselIT avatar Oct 26 '21 21:10 CaselIT

all of these words are supposed to be available from sqlalchemy.sql.expression, and sqlalchemy.sql.elements is an internal name. so importing from "sqlalchemy.sql.expression" should produce a name that does not produce mypy errors.

zzzeek avatar Oct 26 '21 21:10 zzzeek

Should all be updated in Sqlalchemy then?

CaselIT avatar Oct 26 '21 21:10 CaselIT

that was my immediate impression but with all things mypy there is some uncertainty

zzzeek avatar Oct 26 '21 21:10 zzzeek

I'll update both then.

Is these some other module that may be in the same situation and should.be checked?

CaselIT avatar Oct 26 '21 21:10 CaselIT

I think any module that is in the docs should export the names it documents https://docs.sqlalchemy.org/en/14/genindex.html

rectalogic avatar Oct 26 '21 21:10 rectalogic

doesnt apply to the stubs here, but for SQLAlchemy itself which will be retiring the separate stubs package, there seems to be some debate over if a Python package w/ py.typed needs to do explicit re-exports: https://github.com/python/mypy/issues/8754

zzzeek avatar Jan 24 '22 22:01 zzzeek