octabytes / FireO

Google Cloud Firestore modern and simplest convenient ORM package in Python. FireO is specifically designed for the Google's Firestore
https://fireo.octabyte.io
Apache License 2.0
247 stars 29 forks source link

FireO 2.2.1 issues with pylance and co #219

Open ingwinlu opened 9 months ago

ingwinlu commented 9 months ago

I am currently evaluating if I want to replace my own ORM solution based on pydantic models and some inheritance of base models (very simple, basically can only do crud) with FireO.

However my python language server / tooling complains basically all the time when using FireO:

class User(Model):
    name = TextField()
    age = NumberField()

u = User()
u.name = "Azeem"
u.age = 26
u.save()

Errors (removed filenames):

[{
    "owner": "_generated_diagnostic_collection_name_#10",
    "code": {
        "value": "reportGeneralTypeIssues",
        "target": {
            "$mid": 1,
            "path": "/microsoft/pyright/blob/main/docs/configuration.md",
            "scheme": "https",
            "authority": "github.com",
            "fragment": "reportGeneralTypeIssues"
        }
    },
    "severity": 8,
    "message": "Cannot assign member \"name\" for type \"User\"\n  Expression of type \"Literal['Azeem']\" cannot be assigned to member \"name\" of class \"User\"\n    Member \"__set__\" is unknown\n    \"Literal['Azeem']\" is incompatible with \"TextField\"",
    "source": "Pylance",
    "startLineNumber": 58,
    "startColumn": 3,
    "endLineNumber": 58,
    "endColumn": 7
},{
    "owner": "_generated_diagnostic_collection_name_#10",
    "code": {
        "value": "reportGeneralTypeIssues",
        "target": {
            "$mid": 1,
            "path": "/microsoft/pyright/blob/main/docs/configuration.md",
            "scheme": "https",
            "authority": "github.com",
            "fragment": "reportGeneralTypeIssues"
        }
    },
    "severity": 8,
    "message": "Cannot assign member \"age\" for type \"User\"\n  Expression of type \"Literal[26]\" cannot be assigned to member \"age\" of class \"User\"\n    Member \"__set__\" is unknown\n    \"Literal[26]\" is incompatible with \"NumberField\"",
    "source": "Pylance",
    "startLineNumber": 59,
    "startColumn": 3,
    "endLineNumber": 59,
    "endColumn": 6
}]

I also noticed that things like the .get method seem to indicate wrong types even though when I look at the code I see typevars that should be able to let the language server know the object's class when calling the funciton:

image

Hence my question: Am I doing something completely wrong? Is the type hinting working as expected?

ADR-007 commented 9 months ago

Hi! If you want to have (more) proper typing, you may wish to use TypedModel:

class User(TypedModel):
    name: str
    age: int | None # if nullable

About type annotation for .get method - we need to update the FireO code or write a .pyi file for it.

I would recommend you improve FireO instead of writing your own solution. There are quite a lot of things you will need to implement otherwise. I made many fixes/improvements to this library when I used it on one of my projects. It worked fine and covered my needs.

If you are using FastAPI or FactoryBoy, I can provide you with some code snippets related to it. (or we can add it to this repo or make a new one)

ingwinlu commented 9 months ago

Thanks for the response.

If you want to have (more) proper typing, you may wish to use TypedModel:

That indeed works rather well. Only downside is you have to ignore things like errors on IDField's:

class User(TypedModel):
    id: str = IDField(required=True)  # type: ignore
    name: str
    age: int

But I still consider this usable.

As for the Manager it seems the type annotations on Model do not provide any benefit -> pylance is smart enough to see that the metaclasses override it with unknown types. One workaround would be to reassign the collection attribute after you define the class: User.collection = Manager[User](model_cls=User) image

I would recommend you improve FireO instead of writing your own solution. There are quite a lot of things you will need to implement otherwise. I made many fixes/improvements to this library when I used it on one of my projects. It worked fine and covered my needs.

I agree. The other system is just already running and fine for the current use case. But as soon as I want to do subcollections I would need to touch it.


I would suggest closing this issue and reopening two other more specific issues to be worked on:

  1. Field assignment breaks typing (can probably look at pydantic to see how they solve the issue there)
  2. Extend / Fix the Manager type inference in the meta model class