Complete stubs for SQLAlchemy

[JelleZijlstra]

Fixes #974 (hopefully!).

This is intended to fix false positive errors that arise from incomplete stubs.

This is a massive PR that will be hard to review. I did the following (all in separate commits):

• Have stubgen generate stubs and commit them
• Diff the code with the existing code in typeshed, and bring back any improvements (e.g., more specific types) from the existing code.
• Fix the hundreds of mypy errors in the generated stubs.
• Fix up a few more lint and compatibility errors.
  To review this diff, I recommend you look only at the commits after the first one.

@Deimos @lincolnq @Stiivi would you mind testing the stubs from this PR against your codebase to see if you still get false positive errors from mypy?

[Stiivi]

I can’t no longer import some of the top-level modules such as sqlalchemy.exc. Getting an error ”module" has no attribute “exc”. Tried to import others and this is the result. >> is "no attribute" error, the others are fine:

>> 68 a = sqlalchemy.cprocessors 
>> 69 b = sqlalchemy.cresultproxy
>> 70 c = sqlalchemy.cutils      
>> 71 d = sqlalchemy.events      
>> 72 e = sqlalchemy.exc         
   73 f = sqlalchemy.inspection  
>> 74 g = sqlalchemy.interfaces  
   75 h = sqlalchemy.log         
   76 i = sqlalchemy.pool        
>> 77 j = sqlalchemy.processors  
   78 k = sqlalchemy.schema      
   79 l = sqlalchemy.types       

[Stiivi]

Re: module has no attribute.... Works as expected after 00c8608.

[Stiivi]

Super should be Function not object. The auto-generated chain is probably lost around here:

class GenericFunction(util.with_metaclass(_GenericMeta, Function)):
    ...

Otherwise we get type incompatibility errors for things like:

x: sa.ColumnElement
y: sa.ColumnElement
x = sa.sum(y)

Where we try to assign sum type to sa.ColumnElement.

[Stiivi]

Should be @property, sqlalchemy is using @_memoized_property

[Stiivi]

Should stay as @property, sqlalchemy is using @_memoized_property, otherwise we can't access X.columns[name] for "function not being indexable".

[JelleZijlstra]

Thanks for checking these! I'm going to grep for _memoized_property throughout the codebase to find any others where I messed this up.

[gvanrossum]

Hm, I ran this against our codebase and there are some new errors:

• Module 'sqlalchemy.engine.base' has no attribute 'RowProxy' -- source has from sqlalchemy.engine.base import RowProxy

• Too few arguments for "update" of "TableClause" -- source has query = table.update().where(where_clauses).values(**kwargs)

• Ditto for "insert" and "delete"

• "Table" has no attribute "constraints" -- source for constraint in table.constraints:

[JelleZijlstra]

Thanks for checking! I can fix those specific issues, but I'm beginning to think that maybe this PR's approach of mass-creating stubs for all of SQLAlchemy isn't the right one.

[gvanrossum]

Well what's currently in typeshed is also failing for us (just on different APIs).

I'm beginning to think that maybe we should indeed delete the sqlalchemy stubs entirely (as was one option discussed in #974), at least for the time being. The --ignore-missing-modules flag can then be used to silence the imports (possibly even selectively just for sqlalchemy.* in the config file).

[Stiivi]

I think that selectively ignoring whole modules or symbols would be better solution than to drop it completely. It already contains quite useful pieces of information about the library and can serve as a good starting point for further development.

[gvanrossum]

But there's no way to make mypy do that without each app copying a huge section to its mypy.ini file.

[Stiivi]

When we consider dropping sqlalchemy from master typeshed, what do you think about being able to specify --include-typeshed-dir DIR (compare to --custom-typeshed-dir DIR) or keep --custom-typeshed-dir with additional flag --include-default-typeshed or --include-builtin-typeshed ? Then projects can synchronize library-specific typesheds in their $PROJECT/typeshed/$LIB:

cubes/
    cubes/
        *.py
    typeshed/
        sqlalchemy/
            *.pyi
        otherlib/
            *.pyi
    mypy.ini
    setup.py
    ...

Where the typeshed/$LIB might be maintained as git submodules.

The mypy.ini would look like:

[mypy]
custom_typeshed_dir = ./typeshed
include_builtin_typeshed = true

Larger libraries can be then maintained as separate git repositories.

[gvanrossum]

How about we allow the existing flag to have multiple args. Agreed we need to support adding extra typeshed dirs. You could also add it to $MYPYPATH.

[gvanrossum]

I'd like feedback from @matthiaskramm (pytype) and @vlasovskikh (PyCharm) -- what would your preferences be?

[matthiaskramm]

No strong opinions on anything in third_party, from our side. We don't automatically include those. (At best, we use the pyis as a starting point for (manually) annotating one of our third_party packages.)

That said, sqlalchemy has been making trouble for quite some time:
#14
#114
#137
#708
#974
So I'm +1 for getting rid of that.

[Deimos]

I'm not sure if you were still looking for testing/input since it looks like the plan might be to just delete them now, but I'm still seeing quite a few similar errors with these new stubs (though I think it's less than with the currently included ones - I can confirm that if you'd like).

Most of the errors seem to be similar to before though, incompatible types for assignments/return values/arguments where mypy considers a Column incompatible with the base type underlying it. My small example in #974 still demonstrates this issue with setting a String column to a str value detected as an incompatible assignment. Similarly, trying to call any function expecting a str argument with a User.name value would give an incompatible type error on that argument, and so on.

One other error I got with these stubs was:

error: Unexpected keyword argument "naming_convention" for "MetaData"

from a line of code similar to:

metadata = MetaData(naming_convention=NAMING_CONVENTION)

This argument should be valid though: http://docs.sqlalchemy.org/en/latest/core/metadata.html#sqlalchemy.schema.MetaData

[JelleZijlstra]

OK, sounds like we're leaning towards removing the stubs. If so, I'll close this PR and put the code in a separate repo, so that people who want to use it can include it manually.

@Stiivi @gvanrossum I don't think we need to change mypy, since you can just write mypy_path = ... in mypy.ini already.

@Deimos Thanks for checking! I'm pretty sure no stubs will allow that code sample to work; the metaclass is too magical. Did it ever work for you? I'm getting Invalid base class even if I delete all sqlalchemy stubs. I can't reproduce your problem with MetaData either: the argument is there in the stubs from this PR and I didn't get any mypy errors on a test file that uses it.

[JelleZijlstra]

Also, responding to the problems Guido found in his codebase:

Module 'sqlalchemy.engine.base' has no attribute 'RowProxy' -- source has from sqlalchemy.engine.base import RowProxy

On the latest version of SQLAlchemy, from sqlalchemy.engine.base import RowProxy gives me an ImportError. Maybe it worked on a previous version.

Too few arguments for "update" of "TableClause" -- source has query = table.update().where(where_clauses).values(**kwargs)
Ditto for "insert" and "delete"

These are using a decorator that magically inserts an extra argument. I'll grep for it in SQLAlchemy source and remove the arguments in the stubs.

"Table" has no attribute "constraints" -- source for constraint in table.constraints:

This is set in an init (not __init__) method. I'll add it to the stub manually.

[gvanrossum]

OK. Let's move the sqlalchemy to a separate project; whoever wants to own it can, AFAIK. PyCharm can include its own copy of it in their distribution. Can you merge my PR for that?

[JelleZijlstra]

Will do.

[vlasovskikh]

PyCharm currently bundles Typshed, but we use only a few stubs from it at the moment. We could bundle sqlachemy stubs from any other location as well. вт, 28 марта 2017 г. в 17:23, Jelle Zijlstra <notifications@github.com>:

Will do. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1094 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHvq29zvf0I-k75AG0U-8HUghkOnbAjks5rqSYMgaJpZM4Mo8eb> .

-- Andrey Vlasovskikh Web: http://pirx.ru/

[JelleZijlstra]

I just created https://github.com/JelleZijlstra/sqlalchemy-stubs. Interested people can add this repo to their MYPYPATH directly.

@Stiivi if you like, I can add you as a collaborator to my repo. I'll also accept pull requests.

[Stiivi]

@JelleZijlstra thank you, would be more than happy.