Skip to content

Conversation

@RinCodeForge927
Copy link
Contributor

This PR implements the TODOs mentioned in PYTHON-2442 by replacing manual dictionary construction in _options_dict() with calls to _asdict() in both CodecOptions and JSONOptions. This is more idiomatic as these classes inherit from NamedTuple (or use its interface).

Replaced manual dictionary creation with the more idiomatic _asdict() method in both CodecOptions and JSONOptions.
@RinCodeForge927 RinCodeForge927 requested a review from a team as a code owner December 30, 2025 14:22
@blink1073
Copy link
Member

Hi @RinCodeForge927, thank you for the fix! Looking at the original commit, I think we can remove the private method _options_dict entirely. I verified we don't use it in motor.

As suggested in PR review, _options_dict is redundant. Replaced its usage in JSONOptions.with_options with _asdict().
@RinCodeForge927
Copy link
Contributor Author

Thanks for the suggestion! I've removed _options_dict entirely and updated with_options to use _asdict directly. Please take a look.

@blink1073
Copy link
Member

Looks like at least one more usage needs to be updated based on the docs build failure:

      File "/home/docs/checkouts/readthedocs.org/user_builds/pymongo/envs/2670/lib/python3.11/site-packages/bson/codec_options.py", line 486, in with_options
        opts = self._options_dict()
               ^^^^^^^^^^^^^^^^^^
    AttributeError: 'CodecOptions' object has no attribute '_options_dict'

Following reviewer feedback, the private method _options_dict has been removed from both CodecOptions and JSONOptions. Call sites have been updated to use _asdict() directly.
@RinCodeForge927
Copy link
Contributor Author

I've cleaned up the remaining references to _options_dict in codec_options.py and confirmed that all tests pass locally. The method is now completely removed. Thank you!

@blink1073 blink1073 changed the title Refactor: use _asdict() in _options_dict() (PYTHON-2442) PYTHON-2442: Refactor: use _asdict() in _options_dict() Dec 30, 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, thank you!

@RinCodeForge927
Copy link
Contributor Author

Thank you for the review! I'm very happy to contribute a small improvement to such a great project. Glad I could help!

@blink1073 blink1073 merged commit 6585d9c into mongodb:master Dec 30, 2025
72 of 74 checks passed
@RinCodeForge927 RinCodeForge927 deleted the refactor/use-asdict-for-options branch December 31, 2025 05:17
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.

2 participants