-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Remove unused cimports, fix #22067 #22087
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
cdef int64_t _ordinal_from_fields(year, month, quarter, day, | ||
hour, minute, second, freq): | ||
cdef int64_t _ordinal_from_fields(int year, int month, quarter, int day, | ||
int hour, int minute, int second, freq): |
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.
This is what fixed 22067. year and month were getting an Int64Index.
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.
can you add a test for #22067 ?
@@ -1920,8 +1920,8 @@ class Period(_Period): | |||
return cls._from_ordinal(ordinal, freq) | |||
|
|||
|
|||
cdef int64_t _ordinal_from_fields(year, month, quarter, day, | |||
hour, minute, second, freq): | |||
cdef int64_t _ordinal_from_fields(int year, int month, quarter, int day, |
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.
does quarter not need a type?
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.
It can be None
.
minor comments and pls rebase |
Hello @jbrockmendel! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on July 29, 2018 at 20:57 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #22087 +/- ##
=======================================
Coverage 92.07% 92.07%
=======================================
Files 170 170
Lines 50690 50690
=======================================
Hits 46672 46672
Misses 4018 4018
Continue to review full report at Codecov.
|
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.
rebase & comments, lgtm. ping on green.
# GH#22067, check we don't get warnings about silently ignored errors | ||
dti = date_range('2017-01-01', '2018-01-01', freq='B') | ||
|
||
c = StringIO() |
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.
use tm.capture_stderr
Ping |
thanks! |
Fixes #22067, but no clear way to test it.