djrobstep / migra

Like diff but for PostgreSQL schemas
https://databaseci.com/docs/migra
The Unlicense
2.9k stars 123 forks source link

PEP 484/Mypy type annotations? #114

Open SKalt opened 4 years ago

SKalt commented 4 years ago

Would you be interested in using mypy to validate PEP 484 type annotations as a part of migra's CI? If so, what syntax would you find acceptable? There's: 1) My first choice, python 3.5+ native syntax:

from typing import Union
def foo(a: str, b: int) -> Union[int, str]:
    return b if b else a

2) the other option, python 2.7+ inline comments

def foo(
    a, # type: str
    b # type: int
):
     return b if b else a

3) shipping .pyi stub files, which can't be used to check the migra codebase.

I'd be willing to take this on over the weekend.

djrobstep commented 4 years ago

Hey, this would be awesome! I've been meaning to get these added to the codebase for a while now.

Agree with the preference for the most modern syntax.

Worth noting that while the codebase supports python 2 at the moment, I've been planning to bite the bullet, stop supporting it, clean up the codebase, and take advantage of some 3-only features like dataclasses and natively ordered dictionaries. So don't worry about supporting old versions.

Excited to see how the code looks with annotations added!

SKalt commented 4 years ago

Question barrage incoming. I've been able to narrow down the type errors over on my branch [linked], but I'm unable to figure out the following:

djrobstep commented 4 years ago

Cheers for raising all these issues, lots of room for improvement here. Things are a bit hectic for me over the next few days so won't have time to address all these issues immediately, but a few quick ones:

SKalt commented 4 years ago

update: I'm currently stuck working adding type annotations to the stuff that reqires sqlalchemy internals. Turns out typeshed does not include sqlalchemy stubs. Instead, there are potentially-usable stubs at https://github.com/dropbox/sqlalchemy-stubs, which is in alpha.

Once again, I should have time to make progress this weekend.

SKalt commented 4 years ago

I managed to eliminate type errors within migra, but making stubs for sqlbag and schemainspect hasn't got me far. It will likely be easier to add typing to those modules themselves.

Also, I've hit a wall trying to type callbacks. I had to nuke the great metaprogramming in changes.py; I'd like to write something like

# clear pseudocode

class Callback(Protocol): # use a callback protocol as the method type
    def __call__(self, specific: typed = defaulted, keyword: ars = defaulted) -> Statements: ...

def make_partial(name: Literal["schemas"]) -> Callback:
    def callback(self: 'Changes', *args, **kwargs) -> Statements:
        return statements_for_changes(getattr(self, name), *args, **kwargs)

class Changes():
    schemas = make_partial("schemas")  # should be a method with type Callback

However, the self arg in the methods cause mypy to vomit, so I'm stuck with repetitive method definitions.