Skip to content

Conversation

NoahStapp
Copy link
Contributor

We already supported hint for distinct() through kwargs, but we didn't mention it. Now we explicitly support it similarly to the other Collection methods.

@NoahStapp NoahStapp requested a review from blink1073 March 25, 2025 14:44
blink1073
blink1073 previously approved these changes Mar 25, 2025
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@blink1073
Copy link
Member

Ah, except typing:

pymongo/synchronous/collection.py:3165: error: Incompatible types in assignment
(expression has type "Union[str, dict[str, Any]]", target has type "str") 
[assignment]
                cmd["hint"] = hint
                              ^~~~
pymongo/asynchronous/collection.py:3172: error: Incompatible types in
assignment (expression has type "Union[str, dict[str, Any]]", target has type
"str")  [assignment]
                cmd["hint"] = hint

@NoahStapp
Copy link
Contributor Author

Ah, except typing:

pymongo/synchronous/collection.py:3165: error: Incompatible types in assignment
(expression has type "Union[str, dict[str, Any]]", target has type "str") 
[assignment]
                cmd["hint"] = hint
                              ^~~~
pymongo/asynchronous/collection.py:3172: error: Incompatible types in
assignment (expression has type "Union[str, dict[str, Any]]", target has type
"str")  [assignment]
                cmd["hint"] = hint

Fixed!

@NoahStapp NoahStapp requested a review from blink1073 March 25, 2025 14:53
@NoahStapp NoahStapp merged commit 4403169 into mongodb:master Mar 25, 2025
32 of 34 checks passed
if hint is not None:
if not isinstance(hint, str):
hint = helpers_shared._index_document(hint)
cmd["hint"] = hint # type: ignore[assignment]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need to add tests for this like we do for other helpers that accept hint.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests besides the spec tests? The spec tests that verify this change were already present in our repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes because we have python specific behavior, like accepting hint="str" and hint=[pairs].

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.

3 participants