PYTHON-3493 Bulk Write InsertOne Should Be Parameter Of Collection Type #1106

juliusgeo · 2022-11-02T20:08:32Z

No description provided.

pymongo/operations.py

blink1073 · 2022-11-03T12:09:11Z

  File "/home/runner/work/mongo-python-driver/mongo-python-driver/pymongo/operations.py", line 22, in <module>
    from pymongo.typings import RawBSONDocument, _CollationIn, _DocumentType, _Pipeline
ImportError: cannot import name 'RawBSONDocument' from 'pymongo.typings' (/home/runner/work/mongo-python-driver/mongo-python-driver/pymongo/typings.py)

That import is only during type checking, you'll have to import from the raw_bson module directly.

pymongo/collection.py

blink1073 · 2022-11-03T14:07:50Z

It looks like typing_extensions is still not installed in EG:

[2022/11/03 13:33:05.363] ERROR: name 'Movie' is not defined (NameError)
 [2022/11/03 13:33:05.363] Traceback (most recent call last):
[2022/11/03 13:33:05.363]   File "/data/mci/ad80d9537e77ee1c525bef8a857fbff0/src/test/test_mypy.py", line 121, in test_bulk_write
[2022/11/03 13:33:05.363]     requests: List[InsertOne[Movie]] = [InsertOne(Movie(name="American Graffiti", year=1973))]
[2022/11/03 13:33:05.363] NameError: name 'Movie' is not defined

test/test_session.py

test/test_server_selection.py

pymongo/typings.py

blink1073

LGTM!

ShaneHarvey · 2022-11-08T23:13:12Z

doc/examples/type_hints.rst

+  >>> assert result["_id"]
+
+This same typing scheme works for all of the insert methods (`insert_one`, `insert_many`, and `bulk_write`). For `bulk_write`,
+both `InsertOne/Many` and `ReplaceOne/Many` operators are generic.


There's no such thing as InsertMany or ReplaceMany.

Also can you make these rst links to the methods and classes (:meth:...).

@juliusgeo ReplaceMany should be removed, there is no ReplaceMany operator

ShaneHarvey · 2022-11-09T19:03:31Z

doc/examples/type_hints.rst

+
+This same typing scheme works for all of the insert methods (:meth:`~pymongo.collection.Collection.insert_one`,
+:meth:`~pymongo.collection.Collection.insert_many`, and :meth:`~pymongo.collection.Collection.bulk_write`).
+For `bulk_write`, :py:class:`~pymongo.operations.InsertOne`, :py:class:`~pymongo.operations.ReplaceOne`, and


:py:class: -> :class:

ShaneHarvey · 2022-11-09T19:46:57Z

doc/examples/type_hints.rst

+This same typing scheme works for all of the insert methods (:meth:`~pymongo.collection.Collection.insert_one`,
+:meth:`~pymongo.collection.Collection.insert_many`, and :meth:`~pymongo.collection.Collection.bulk_write`).
+For `bulk_write`, :class:`~pymongo.operations.InsertOne`, :class:`~pymongo.operations.ReplaceOne`, and
+:class:`~pymongo.operations.UpdateMany` operators are generic.


UpdateMany is not generic.

It is made generic in this PR. Line 378 of operations.py. The name of the class is elided from the GitHub diff view. That would mean I need to add tests, though. I will do that.

ShaneHarvey · 2022-11-09T19:48:30Z

pymongo/operations.py

@@ -374,7 +375,7 @@ class UpdateMany(_UpdateOp):
    def __init__(
        self,
        filter: Mapping[str, Any],
-        update: Union[Mapping[str, Any], _Pipeline],
+        update: Union[_DocumentType, RawBSONDocument, _Pipeline],


Woah, the update doc for UpdateOne and UdateMany shouldn't accept the _DocumentType. The update is a mapping like this {"$inc": ...}, not a full doucment.

Ah, I see. Those were added in without tests, hence why that wasn't noticed. I will remove those changes.

ShaneHarvey · 2022-11-09T20:06:51Z

test/test_database.py

@@ -201,7 +201,7 @@ def test_list_collection_names_filter(self):
        db.capped.insert_one({})
        db.non_capped.insert_one({})
        self.addCleanup(client.drop_database, db.name)
-
+        filter: dict


This is a type hint for mypy so that this error does not occur:

(python3.11) ➜ mongo-python-driver git:(PYTHON-3493) mypy --install-types --non-interactive --disable-error-code var-annotated --disable-error-code attr-defined --disable-error-code union-attr --disable-error-code assignment --disable-error-code no-redef --disable-error-code index --allow-redefinition --allow-untyped-globals --exclude "test/mypy_fails/*.*" test test/test_database.py: note: In member "test_list_collection_names_filter" of class "TestDatabase": test/test_database.py:208: error: Argument "filter" to "list_collection_names" of "Database" has incompatible type "object"; expected "Optional[Mapping[str, Any]]" [arg-type] names = db.list_collection_names(filter=filter) ^~~~~~ Found 1 error in 1 file (checked 121 source files)

Oh I see. I thought it was a copy/past error. Down below there's already filter: Any. Can we define this once? perhaps filter: Union[None, dict] would be better?

Yup, that works.

ShaneHarvey · 2022-11-09T20:07:53Z

test/test_bulk.py

@@ -347,7 +347,7 @@ def test_bulk_write_no_results(self):

    def test_bulk_write_invalid_arguments(self):
        # The requests argument must be a list.
-        generator = (InsertOne({}) for _ in range(10))
+        generator = (InsertOne[dict]({}) for _ in range(10))


Why are these changes required? This seems annoyingly verbose for users.

This is required because otherwise mypy complains:

(python3.11) ➜ mongo-python-driver git:(PYTHON-3493) ✗ mypy --install-types --non-interactive --disable-error-code var-annotated --disable-error-code attr-defined --disable-error-code union-attr --disable-error-code assignment --disable-error-code no-redef --disable-error-code index --allow-redefinition --allow-untyped-globals --exclude "test/mypy_fails/*.*" test test/test_bulk.py: note: In member "test_numerous_inserts" of class "TestBulk": test/test_bulk.py:299: error: Argument 1 to "InsertOne" has incompatible type "Dict[<nothing>, <nothing>]"; expected "RawBSONDocument" [arg-type] requests = [InsertOne({}) for _ in range(n_docs)] ^~ Found 1 error in 1 file (checked 121 source files)

I'm not sure why it doesn't resolve to the bound of the TypeVar.

For the same reason we need a type annotation on MongoClient

Oh I see, it's just empty/untyped dictionary. A non-empty dict works without a manual type annotation:

InsertOne({}) # Argument 1 to "InsertOne" has incompatible type "Dict[<nothing>, <nothing>]"; expected "RawBSONDocument" InsertOne({"a": 1}). # This works

It's a shame the error message is so opaque.

blink1073 · 2022-11-10T19:05:07Z

doc/examples/type_hints.rst

+For `bulk_write` both :class:`~pymongo.operations.InsertOne` and :class:`~pymongo.operations.ReplaceOne` operators are generic.
+
+.. doctest::
+


This needs the python version guard

Speaking of, please schedule the docs and doctest evergreen tasks when changing things like this.

juliusgeo added 2 commits November 2, 2022 13:07

initial commit

2a9f80b

add test for replaceone

7292fbe

blink1073 requested changes Nov 3, 2022

View reviewed changes

pymongo/operations.py Outdated Show resolved Hide resolved

pymongo/operations.py Outdated Show resolved Hide resolved

pymongo/operations.py Outdated Show resolved Hide resolved

pymongo/operations.py Outdated Show resolved Hide resolved

remove unecessary generics

39b32b2

blink1073 reviewed Nov 3, 2022

View reviewed changes

pymongo/collection.py Outdated Show resolved Hide resolved

juliusgeo added 2 commits November 3, 2022 06:14

fix mro and remove type hint

c9b67ac

fix another mro

032271e

a large number of type changes

cf8f8b0

blink1073 requested changes Nov 3, 2022

View reviewed changes

test/test_session.py Outdated Show resolved Hide resolved

test/test_server_selection.py Outdated Show resolved Hide resolved

juliusgeo force-pushed the PYTHON-3493 branch from f92e4cc to cf8f8b0 Compare November 3, 2022 20:06

juliusgeo added 7 commits November 3, 2022 13:58

fix autoformatter and get rid of uneeded cast

3d0ae16

fix autoformatter mangling again

1a9e5d7

fix annotation

46cc1c7

fix evg failures and try to address mypy failure

dd44e38

forgot an import

c16532e

change type ignore

9c26531

actually fix the true error

c3b1226

juliusgeo commented Nov 7, 2022

View reviewed changes

pymongo/typings.py Show resolved Hide resolved

remove unused import

fbb7862

juliusgeo marked this pull request as ready for review November 7, 2022 17:01

juliusgeo requested a review from ShaneHarvey as a code owner November 7, 2022 17:01

juliusgeo requested a review from blink1073 November 7, 2022 17:01

juliusgeo added 4 commits November 8, 2022 11:09

add more testing, add notes

be764e5

fix pre-commit

a8afc6a

simplify assertion

e1938c4

change comment

c205357

blink1073 requested changes Nov 8, 2022

View reviewed changes

pymongo/typings.py Show resolved Hide resolved

change to docstring

a0ece18

juliusgeo requested a review from blink1073 November 8, 2022 20:29

blink1073 approved these changes Nov 8, 2022

View reviewed changes

ShaneHarvey requested changes Nov 8, 2022

View reviewed changes

add rst links etc

62f4918

juliusgeo requested a review from ShaneHarvey November 9, 2022 16:24

forgot to add ReplaceMany

6d12caa

ShaneHarvey reviewed Nov 9, 2022

View reviewed changes

juliusgeo added 2 commits November 9, 2022 11:05

remove py

0d4f9bd

it was updatemany, not replacemany oops

b68ed1b

ShaneHarvey requested changes Nov 9, 2022

View reviewed changes

juliusgeo added 2 commits November 9, 2022 11:57

revert updatemany changes

b0b7a81

fix docs

6328f59

ShaneHarvey requested changes Nov 9, 2022

View reviewed changes

combine the two type hints

65d18bf

ShaneHarvey approved these changes Nov 9, 2022

View reviewed changes

blink1073 approved these changes Nov 9, 2022

View reviewed changes

juliusgeo and others added 4 commits November 10, 2022 10:12

Merge branch 'master' into PYTHON-3493

99e20a7

fix merge errors

f153b45

fix another malformed merge

371b474

had weird failures locally for some reason

71f9370

juliusgeo requested review from ShaneHarvey and blink1073 November 10, 2022 18:42

blink1073 requested changes Nov 10, 2022

View reviewed changes

add doctest guard

be4c899

blink1073 approved these changes Nov 10, 2022

View reviewed changes

ShaneHarvey approved these changes Nov 10, 2022

View reviewed changes

juliusgeo merged commit 92e6150 into mongodb:master Nov 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PYTHON-3493 Bulk Write InsertOne Should Be Parameter Of Collection Type #1106

PYTHON-3493 Bulk Write InsertOne Should Be Parameter Of Collection Type #1106

juliusgeo commented Nov 2, 2022

blink1073 commented Nov 3, 2022

blink1073 commented Nov 3, 2022

blink1073 left a comment

ShaneHarvey Nov 8, 2022

ShaneHarvey Nov 8, 2022

blink1073 Nov 9, 2022

ShaneHarvey Nov 9, 2022

ShaneHarvey Nov 9, 2022

juliusgeo Nov 9, 2022 •

edited

ShaneHarvey Nov 9, 2022 •

edited

juliusgeo Nov 9, 2022 •

edited

ShaneHarvey Nov 9, 2022

juliusgeo Nov 9, 2022

ShaneHarvey Nov 9, 2022

juliusgeo Nov 9, 2022

ShaneHarvey Nov 9, 2022

juliusgeo Nov 9, 2022

blink1073 Nov 9, 2022

ShaneHarvey Nov 9, 2022

blink1073 Nov 10, 2022

ShaneHarvey Nov 10, 2022

		For `bulk_write` both :class:`~pymongo.operations.InsertOne` and :class:`~pymongo.operations.ReplaceOne` operators are generic.

		.. doctest::

PYTHON-3493 Bulk Write InsertOne Should Be Parameter Of Collection Type #1106

PYTHON-3493 Bulk Write InsertOne Should Be Parameter Of Collection Type #1106

Conversation

juliusgeo commented Nov 2, 2022

blink1073 commented Nov 3, 2022

blink1073 commented Nov 3, 2022

blink1073 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusgeo Nov 9, 2022 • edited

Choose a reason for hiding this comment

ShaneHarvey Nov 9, 2022 • edited

Choose a reason for hiding this comment

juliusgeo Nov 9, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliusgeo Nov 9, 2022 •

edited

ShaneHarvey Nov 9, 2022 •

edited

juliusgeo Nov 9, 2022 •

edited