-
Notifications
You must be signed in to change notification settings - Fork 1.1k
PYTHON-3376/PYTHON-3378 Update FAQ about OverflowError when decoding out of range datetimes #1025
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
Conversation
doc/faq.rst
Outdated
the default behavior and | ||
:attr:`~bson.datetime_ms.DatetimeConversionOpts.DATETIME_MS`, returning | ||
:class:`~datetime.datetime`s when possible, and | ||
:class:`~bson.datetime_ms.DatetimeMS` when representations are out-of-range. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than duplicating the behavior of each DatetimeConversionOpts, what if we just suggest using DATETIME_AUTO
and then link to the DatetimeConversionOpts example page for the rest?
There's a broken link to Edit: fixed |
doc/faq.rst
Outdated
@@ -502,6 +518,7 @@ just that field:: | |||
|
|||
>>> cur = coll.find({}, projection={'dt': False}) | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra space?
Started making edits to |
doc/faq.rst
Outdated
For other options, please refer to | ||
:class:`~bson.codec_options.DatetimeConversionOpts`. | ||
|
||
If we need to use the default decoding behavior with datetimes not supported by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest changing from "If we need to use the default decoding behavior ... filter out documents ..." -> "Another option that does not involve setting datetime_conversion
is to to filter out documents ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM assuming the doctests pass.
doc/examples/datetimes.rst
Outdated
>>> decode(x, codec_options=codec_clamp) | ||
{'x': datetime.datetime(1970, 1, 1, 0, 0)} | ||
>>> decode(x, codec_options=codec_clamp) | ||
{'x': DatetimeMS(-2**62)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be datetime.datetime(1, 1, 1, 0, 0)
(min)?
FYI you can run the doctests locally by running |
Added to the FAQ, and tweaked another doc. Will have to be tweaked again after renaming
DatetimeConversionOpts
.