Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Type Hints #354

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ThibaultLemaire
Copy link

Closes #233

@ThibaultLemaire
Copy link
Author

So this is part of a little proof of concept that I'm working on: Can you leverage mypy to detect errors in your mongo queries?

For exemple, with my current code I can detect this:

from umongo import Document
from umongo.fields import StrField

class MyDocument(Document):
    known_field = StrField(required=True)

    
cursor = MyDocument.find({"known_field": 123, "unknown_field": "string"}) # E: main.MyDocument has no field "unknown_field"  [misc]

So right now I just implemented the bare minimum to get my code working and I'm testing manually, but my question is:

How can we test type hints automatically?

  • Run mypy on the examples?
  • Run mypy on the unit tests?
  • ...

I'm open to suggestions.

@ThibaultLemaire
Copy link
Author

ThibaultLemaire commented Jul 7, 2021

For the record, running mypy on the current codebase yields 44 errors which I am not going to list here, though I will note that most of them are caused by missing annotations on dependencies (e.g. error: Cannot find implementation or library stub for module named "pymongo").

Here is the list:

pymongo
pymongo.errors
pymongo.database
pymongo.cursor
bson
motor.motor_asyncio
marshmallow
twisted.internet.defer
txmongo
txmongo.database
mongomock.database
mongomock.collection

pymongo and bson might be fixed with pymongo-stubs but I do not have a solution for the rest which is an other reason why I prefer to go with stubs for umongo

EDIT: My bad I just didn't have the dependencies in my .venv. The error when mypy can't find type hints for a package is actually this one: error: Skipping analyzing "pymongo": found module but no type hints or library stubs

The list of packages missing type hints (after installing pymongo-stubs) is the following:

txmongo
txmongo.database
motor.motor_asyncio

Thibault Lemaire added 4 commits July 7, 2021 14:39
With some tweaks so

`mypy umongo/`

runs without errors.

Those are only basic stubs they should be changed to appropriate types later
And steal some signatures from marshmallow's source code
@ThibaultLemaire
Copy link
Author

ThibaultLemaire commented Jul 8, 2021

Ok, so, now that I have a more complete set of stubs I'm hitting what @talwrii warned about. That is: metaprogramming.

I.e. my previous example was incorrect: without registering my Document to an instance I shouldn't be able to call find on it.

Here is an updated example (note the @instance.register decorator):

from umongo import Document
from umongo.fields import StrField
from umongo.frameworks import MotorAsyncIOInstance

instance = MotorAsyncIOInstance()

@instance.register  # Changes the parent type to MotorAsyncIODocument
class MyDocument(Document):
  known_field = StrField(required=True)

    
MyDocument.find()  # With motor, this isn't a cursor, but a coroutine that returns a cursor, and this is where mypy can remind us to await it.

Naively, this shouldn't be too hard to type correctly. A decorator boils down to a function that takes a function/class definition and returns an other function/class definition. So typing the decorator properly should be enough for mypy to work its magic, right? This is what I tried:

# umongo/frameworks/motor_asyncio.pyi
...

class MotorAsyncIOInstance(Instance): 
    ...
    def register(self, template: Type[DocumentTemplate]) -> Type[MotorAsyncIODocument]: ...

Except that

class decorators are handled differently than function decorators in mypy: decorating a class does not erase its type

and here is what I get from mypy when checking the previous code:

main:12: error: "Type[MyDocument]" has no attribute "find"  [attr-defined]

Which is true for a umongo.document.DocumentTemplate but not for a umongo.frameworks.motor_asyncio.MotorAsyncIODocument which is what it actually is at runtime...

I suppose a mypy plugin could work that out and fix the situation, but that feels a bit dirty.

@ThibaultLemaire
Copy link
Author

I originally planned to write a mypy plugin that could lint pymongo queries on several ODMs (not only umongo but also mongoengine, or pydantic-odm) but maybe that was a bit ambitious, and I am short on time to work on all of this. (This is why I needed this PR initially)

This is what it is currently able to do:

from umongo import Document
from umongo.fields import StrField
from umongo.frameworks import PyMongoInstance

instance = PyMongoInstance()

@instance.register
class MyDocument(Document):
  known_field = StrField(required=True)

    
cursor = MyDocument.find({"known_field": 123, "unknown_field": "string"}) # E: main.MyDocument has no field "unknown_field"  [misc]
reveal_type(cursor) # N: Revealed type is "umongo.frameworks.pymongo.WrappedCursor[main.MyDocument*]"

Note that with only the type stubs I wrote in this PR (so, without the plugin), this is what mypy will yield:

from umongo import Document
from umongo.fields import StrField
from umongo.frameworks import PyMongoInstance

instance = PyMongoInstance()

@instance.register
class MyDocument(Document):
  known_field = StrField(required=True)

    
cursor = MyDocument.find({"known_field": 123, "unknown_field": "string"}) # E: "Type[MyDocument]" has no attribute "find"  [attr-defined]
reveal_type(cursor) # N: Revealed type is "Any"

That's because of a limitation of mypy in how it handles class decorators. So this PR is effectively useless without this plugin...

So because this plugin has somewhat shifted in scope towards being more umongo-specific, I don't believe it makes sense for it to live in a different repository anymore.

I'm thinking I should move it over here and merge it to this PR, maybe in a folder called mypy like what pydantic does? What do you think @lafrech ?

@msenol86
Copy link

msenol86 commented Nov 5, 2021

any progress on this? we are using fastapi and we are searching for an async pydantic-mongo-odb.

@ThibaultLemaire
Copy link
Author

I have since changed jobs and this is no longer relevant to my current job, so I'm sorry but I won't be working on it anymore (however anyone is welcome to pick up where I left off, and ask for information if there's something I overlooked).

If you are looking for a Pydantic<->Mongo ODM I think you should look into pydantic-odm if you're OK with handling indexes yourself. It's a bit lacking in documentation but IIRC the API is straight-forward enough that you can figure it out for yourself by poking around. (uMongo uses Marshmallow internally which isn't ideal if in the end you want Pydantic. You'd have to go through BSON->Marshmallow->Pydantic and back again.)

@msenol86
Copy link

msenol86 commented Nov 8, 2021

@ThibaultLemaire I think this ODM is better than pydantic-odm, what do you think?

@ThibaultLemaire
Copy link
Author

Indeed. I wish it had existed back when I was making my own search.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for type hints
2 participants