REF: use shareable code for DTI/TDI.insert #30806
Conversation
|
Hello @jbrockmendel! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-01-08 18:34:47 UTC |
|
Will need an API-breaking whatsnew for changing the type of the exception. Though to confirm: TypeErrors are typically raised when the argument has an incorrect type. In this case, the type looks correct, since it's a |
| if fill_val.tz: | ||
| with pytest.raises(ValueError, match=msg): | ||
| msg = "Cannot compare tz-naive and tz-aware" |
There was a problem hiding this comment.
@TomAugspurger is this what you are talking about? this I think was just wrong before, not an API change.
There was a problem hiding this comment.
Yeah. I see now that we raise TypeError for Timestamp == Timestamp, which I think is incorrect but not worth changing.
I still thing think this merits a release note.
There was a problem hiding this comment.
sure, @jbrockmendel can you add for what is the user viisble change.
There was a problem hiding this comment.
updated with whatsnew + green
|
thanks @jbrockmendel |
xref #30757 should go in before this because it contains the tests. After this, we'll be able to de-duplicate the two methods.