Skip to content

Better error message for OOB result #32499

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Mar 11, 2020
Merged

Better error message for OOB result #32499

merged 6 commits into from
Mar 11, 2020

Conversation

TomAugspurger
Copy link
Contributor

Closes #31774

@TomAugspurger TomAugspurger added this to the 1.0.2 milestone Mar 6, 2020
@TomAugspurger TomAugspurger added the Error Reporting Incorrect or improved errors from pandas label Mar 6, 2020
TomAugspurger
TomAugspurger commented Mar 9, 2020
View reviewed changes
pandas/_libs/tslibs/c_timestamp.pyx Outdated
@@ -287,9 +287,13 @@ cdef class _Timestamp(datetime):
if (PyDateTime_Check(self)
and (PyDateTime_Check(other) or is_datetime64_object(other))):
if isinstance(self, _Timestamp):
other = type(self)(other)
both_timestamps = isinstance(other, _Timestamp)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jbrockmendel can you take another look when you get a chance? Needed to change some things around to ensure that Timestamp - datetime.datetime didn't raise (since we internally convert the datetime to a Timestamp).

Copy link
Member

Choose a reason for hiding this comment

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

this looks like you're avoiding re-calling the constructor if we already have a Timestamp. Is that necessary? The constructor was recently optimized for the already-Timestamp case

Copy link
Member

Choose a reason for hiding this comment

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

never mind, i get it now

Copy link
Member

Choose a reason for hiding this comment

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

maybe add a comment to the effect of "we need to know if we started with both-Timestamps to determine whether to raise on Overflow, see GH#..."

@jbrockmendel
Copy link
Member

LGTM

@TomAugspurger
Copy link
Contributor Author

The doc build failure is unrelated:

2020-03-09T21:49:05.9460052Z WARNING: failed to reach any of the inventories with the following issues:
2020-03-09T21:49:05.9461222Z intersphinx inventory 'https://pandas-gbq.readthedocs.io/en/latest/objects.inv' not fetchable due to <class 'requests.exceptions.HTTPError'>: 502 Server Error: Bad Gateway for url: https://pandas-gbq.readthedocs.io/en/latest/objects.inv
2020-03-09T21:49:09.4756947Z WARNING: failed to reach any of the inventories with the following issues:
2020-03-09T21:49:09.4758680Z intersphinx inventory 'https://dateutil.readthedocs.io/en/latest/objects.inv' not fetchable due to <class 'requests.exceptions.HTTPError'>: 502 Server Error: Bad Gateway for url: https://dateutil.readthedocs.io/en/latest/objects.inv

@TomAugspurger TomAugspurger mentioned this pull request Mar 10, 2020
Closed
@jreback jreback merged commit 650cf74 into pandas-dev:master Mar 11, 2020
@jreback
Copy link
Contributor

jreback commented Mar 11, 2020

very nice @TomAugspurger

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Mar 11, 2020
@TomAugspurger @meeseeksmachine
Backport PR pandas-dev#32499: Better error message for OOB result
5b5a32d
@meeseeksmachine meeseeksmachine mentioned this pull request Mar 11, 2020
Merged
WillAyd pushed a commit that referenced this pull request Mar 11, 2020
@meeseeksmachine
Backport PR #32499: Better error message for OOB result (#32605)
e91f46d
@TomAugspurger TomAugspurger deleted the 31774-message branch March 12, 2020 12:22
SeeminSyed pushed a commit to CSCD01-team01/pandas that referenced this pull request Mar 22, 2020
@TomAugspurger @SeeminSyed
Better error message for OOB result (pandas-dev#32499)
bfc1544
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbrockmendel jbrockmendel jbrockmendel left review comments

Assignees
No one assigned
Labels
Error Reporting Incorrect or improved errors from pandas
Projects
None yet
Milestone
1.0.2
Development

Successfully merging this pull request may close these issues.

REGR: Timestamp subtraction raises TypeError, Non informative
3 participants
@TomAugspurger @jbrockmendel @jreback