ponyorm / pony

Pony Object Relational Mapper
Apache License 2.0
3.61k stars 243 forks source link

Trouble with mypy and 'has no attribute "__iter__"' #479

Open fluffy-critter opened 4 years ago

fluffy-critter commented 4 years ago

Hi, I'm using PonyORM in a project and am trying to make use of mypy to do stricter type checking. However, mypy is running into some issues doing type analysis on pony's classes.

Given a module named model with the following:

from pony import orm

db = orm.Database()
class Entry(db.Entity):
    file_path = orm.Required(str)
    # ... etc ...

First I was running into trouble with type checking around db.Entity. I managed to work around this by declaring:

DbEntity: orm.core.Entity = db.Entity

and then having my entity classes derive from DbEntity instead. However, I'm having great difficulty figuring out how to fix a different error; in code like:

        limit = max(10, orm.get(orm.count(e) for e in model.Entry) * 5)

I'm getting the error:

error: "Type[Entry]" has no attribute "__iter__" (not iterable)

I'm not finding anything in the mypy docs about disabling this check, and I assume this indicates that orm.core.entity isn't declaring an __iter__ statically (which makes sense since it can't know the implementation until db.bind()).

I tried thunking around this by declaring a do-nothing __iter__ on each of the entity classes, like

class Entry(DbEntity):
    def __iter__(self):
        super().__iter__()

but that didn't resolve the error (and that would have probably run afoul of something else at runtime anyway).

I couldn't find anything in the PonyORM docs about supporting mypy, and nothing in the mypy docs about disabling this particular check or hinting that the class is iterable even if it doesn't look like it up front.

Is this something that can be fixed on my end, and/or would it be possible to have PonyORM declare a do-nothing __iter__ on orm.core.Entity that then gets overridden at bind() time?

fluffy-critter commented 4 years ago

Okay, after poking at it some more I figured out that I can do:

        limit = max(10, orm.get(orm.count(e)
                                for e in typing.cast(typing.Iterable, model.Entry)) * 5)

I'd still like to see the do-nothing __iter__ added to the core entity type though (or whatever fix seems more appropriate).

Thanks!

Xandaros commented 2 years ago

I ended up putting an iter function on all my entities that does nothing but change the type to Iterable[MyEntity] like this:

class MyEntity(db.Entity):  # type: ignore
    id = PrimaryKey(int)
    some_value: Required(str)

    @classmethod
    def iter(cls) -> Iterable["MyEntity"]:
        return self

And the type: ignore is there because otherwise it would complain about db.Entity...

It would really be nice if such hacks weren't necessary, but I guess it's not too much of a hassle to use select(ent for ent in MyEntity.iter()) instead of select(ent for ent in MyEntity)

I'm honestly not sure if its fixable with current type checkers. Even manually setting the metaclass to EntityMeta does not fix it and EntityMeta absolute does define __iter__...

Jonesus commented 2 years ago

Hi! If you're still facing issues with this, could you try out the stubs package I made to see if it would work for your needs? https://github.com/Jonesus/pony-stubs

fluffy-critter commented 2 years ago

I gave that a try and it didn't really help anything.

from pony import orm
from typing_extensions import TypeAlias

db = orm.Database()  # pylint: disable=invalid-name
DbEntity: TypeAlias = orm.core.Entity

class GlobalConfig(DbEntity):
    """ Global configuration data """
    key = orm.PrimaryKey(str)
    int_value = orm.Optional(int)

results in an error like:

publ/model.py:25: error: Need type annotation for "key"

Also, mypy complains about using db.Entity instead of the DbEntity alias (with Name "db.Entity" is not defined).

This is on Python 3.8.12 with Pony 0.7.16 and pony-stubs 0.5.0.

Also, just as a note, a more standard name for the types package would be types-pony.

fluffy-critter commented 2 years ago

Oh, and another thing is that orm.core.Entity doesn't seem to provide the implicit id field; for example, I have a model.Entry type that uses the implicit int id, and attempts at accessing model.Entry.id result in e.g.

publ/queries.py:30: error: "Type[Entry]" has no attribute "id"
fluffy-critter commented 2 years ago

That said, the package did help me to remove my type:ignore annotations everywhere, after I added explicit type annotations for the things that couldn't be handled automatically. Thanks!

Jonesus commented 2 years ago

Thanks for all the feedback and issues you created in the repo, I'll answer your concerns there :+1: