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

Make Cursor structurally compatible with Iterable (#11) #12

Closed
wants to merge 1 commit into from

Conversation

ThibaultLemaire
Copy link

@ThibaultLemaire ThibaultLemaire commented Aug 2, 2021

Mypy now considers Cursor and CommandCursor structural subtypes of Iterable

For example, the following code now typechecks correctly:

from pymongo.cursor import Cursor
from typing import Collection, Dict, Any

class MyCursor(Collection[Dict[str, Any]], Cursor):
  ...

Closes #11

Mypy now considers Cursor and CommandCursor structural subtypes of Iterable

For example, the following code now typechecks correctly:

```python
from pymongo.cursor import Cursor
from typing import Collection, Dict, Any

class MyCursor(Collection[Dict[str, Any]], Cursor):
  ...
```
@ThibaultLemaire
Copy link
Author

Currently I'm testing this a bit manually with pytest-mypy-plugins. I see you have some mypy failure checks, do you want me to add some kind of test for this?

@ShaneHarvey
Copy link
Contributor

ShaneHarvey commented Aug 4, 2021

This change looks fine to me so far (besides the trailing whitespace issue) but I just realized that we already have a test that ensures Cursor is iterable:

    def test_cursor_iterable(self) -> None:
        def to_list(iterable: Iterable[Dict[str, Any]]) -> List[Dict[str, Any]]:
            return list(iterable)
        self.coll.insert_one({})
        cursor = self.coll.find()
        docs = to_list(cursor)
        self.assertTrue(docs)

https://github.com/mongodb-labs/pymongo-stubs/blob/0.1.0/test/test_pymongo.py#L56-L62

It seems that the issue is #11 is potentially a bug in mypy. We should report it to them.

class MyCursor(Collection[Dict[str, Any]], Cursor):

What is this line doing exactly? What's the purpose of Collection[Dict[str, Any]]?

@ThibaultLemaire
Copy link
Author

What is this line doing exactly? What's the purpose of Collection[Dict[str, Any]]?

A very good question. I reported #11 as part of a PR I'm making to type umongo: Scille/umongo#354

umongo wraps Cursor to return instances of its Document class. I was thinking of the following kind of use-case:

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)

def main():
  # Works with a list
  generic_function([MyDocument(known_field="a"), MyDocument(known_field="b")])
  # Or a (Wrapped)Cursor
  generic_function(MyDocument.find())

def generic_function(not_a_list: <What's this type?>):
  """ a function that was written with a list in mind, but actually works with a cursor as well """
  for elem in not_a_list:
    ...
  if not_a_list[-1].known_field == "something":
    ...
  # etc.

A good type for not_a_list could be Sequence[MyDocument] for example (but not Collection[MyDocument] though. I don't know why I went with Collection. I think I wanted the one that supported the most operations but failed to realise Cursor actually does not support __len__ and __contains__). As a library user I often take quick looks at the library's source to help in those cases. If I see an explicit inheritance from, say, Sequence then I know I can safely use Sequence.

So this is why I tried to explicitely inherit from Collection.

But discussing with you I realise mypy works really well with structural subtyping and coming from statically typed languages I'm not used to that.

So I don't know what's wrong with mypy, if there's something wrong with mypy, but you certainly pointed out something wrong with what I'm doing and I'm not sure this PR is relevant anymore.

I wanted to spare myself typing all the methods and rely on the standard typing Protocols but that was a bad idea. I'll leave this PR open for now, but rework my PR on umongo and see where it takes me. Thanks.

@ShaneHarvey
Copy link
Contributor

I'm going to close this PR since I think this is a bug in mypy itself. It would be great if you could report it to them.

@ShaneHarvey ShaneHarvey closed this Nov 5, 2021
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.

Make Cursor compatible with Collection[Dict[str, Any]]
2 participants