sqlite-utils icon indicating copy to clipboard operation
sqlite-utils copied to clipboard

devrel/python api: Pylance type hinting

Open chapmanjacobd opened this issue 3 years ago • 4 comments

Pylance is generally pretty good at figuring out stuff but sqlite-utils has some quirks which make type hinting kinda useless. Maybe you don't care but I thought I would bring it to your attention.

For example:

db["subs"].insert_all(subs, pk="index")
Cannot access member "insert_all" for type "View"
  Member "insert_all" is unknown

insert_all and all the other methods show up as a type issues because the program can't know whether something is a View or a Table. Fair enough. But that basically throws all type checking out the window.

pk="index" also shows up as a type issue:

Argument of type "Literal['index']" cannot be assigned to parameter "pk" of type "Default" in function "insert_all"
  "Literal['index']" is incompatible with "Default"

I think this is because DEFAULT is an empty class?

maybe a few small changes could be made to make the library more type-friendly

The interim solution is of course to turn off type hints completely for the line

db["subs"].insert_all(subs, pk="index")  # type: ignore

chapmanjacobd avatar Oct 01 '22 03:10 chapmanjacobd

I do care about this, but I'm not hugely experienced with types yet so I'm open to suggestions about how to do it!

simonw avatar Oct 25 '22 21:10 simonw

I was going to suggest using db.table(name) instead of db[name] - but it looks like that method will have the same problem:

https://github.com/simonw/sqlite-utils/blob/defa2974c6d3abc19be28d6b319649b8028dc966/sqlite_utils/db.py#L497-L506

I could change sqlite-utils so db.table(name) always returns a table and you need to call db.view(name) if you want to access a view - that would require bumping to 4.0 though. I'm not convinced that's the best approach here either.

simonw avatar Oct 25 '22 21:10 simonw

With respect to the typing of Table class itself, my interim solution:

from sqlite_utils.db import Table
def tbl(self, table_name: str) -> Table:
    tbl = self.db[table_name]
    if isinstance(tbl, Table):
        return tbl
    raise Exception(f"Missing {table_name=}")

With respect to @chapmanjacobd concern on the DEFAULT being an empty class, have also been using # type: ignore, e.g.

@classmethod
def insert_list(cls, areas: list[str]):
    return meta.tbl(meta.Areas).insert_all(
        ({"area": a} for a in areas), ignore=True  # type: ignore
    )

justmars avatar Oct 28 '22 03:10 justmars

Would love to put our heads together for improvements here.

I think anything that is argname=DEFAULT needs to be typed as argname: str | Default = DEFAULT (replacing str with the appropriate type(s)). We may be able to get clever and tie the types to that key in the _defaults dict (definitely possible in Typescript, but I'm less familiar with advanced python types).

Right now, all args are typed as Default, which means all callers get type errors.

As for table/view, given that they don't have the same methods, it would be nice to be able to get one or the other specifically.

xavdid avatar May 03 '23 05:05 xavdid