-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
remove unused time conversion funcs #17711
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
pandas/_libs/index.pyx
Outdated
@@ -415,12 +406,12 @@ cdef class DatetimeEngine(Int64Engine): | |||
if not self.is_unique: | |||
return self._get_loc_duplicates(val) | |||
values = self._get_index_values() | |||
conv = _to_i8(val) |
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.
isn't this a cimport?
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.
At the moment it is cdef
d in _libs.index
. This replaces it with a call to tslib.pydt_to_i8
. As noted in the OP, pydt_to_i8
is currently just def
, but in the PR is made cpdef
so that the c version can be called here (tslib
is cimported into _libs.index
)
Haven't gotten a handle on why, but using So for the time being, this moves |
@@ -3436,7 +3436,24 @@ def cast_to_nanoseconds(ndarray arr): | |||
return result | |||
|
|||
|
|||
def pydt_to_i8(object pydt): | |||
cdef inline _to_i8(object val): |
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.
you need a return type (int64_t)
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.
There are cases in which it returns the object
input.
try: | ||
return val.value | ||
except AttributeError: | ||
if is_datetime64_object(val): |
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.
just Timestamp(val).value
is prob enough here, this looks like really old conversion code
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.
did u fix for this?
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.
See #17711 (comment)
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.
so just change this to (to remove all of this code)
try
return val.value
except AttributeError:
if not is_string_object(val):
return Timestamp(val).value
return val
of maybe is isinstance(val, (np.datetime64, datetime))
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.
Implemented this and it doesn't appear to have broken anything.
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.
great, ok merging this. would still like to remove this routing (in favor of pydt_to_i8 but can do that in another PR.
Codecov Report
@@ Coverage Diff @@
## master #17711 +/- ##
==========================================
- Coverage 91.27% 91.23% -0.05%
==========================================
Files 163 163
Lines 49766 49783 +17
==========================================
- Hits 45423 45418 -5
- Misses 4343 4365 +22
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #17711 +/- ##
==========================================
- Coverage 91.27% 91.24% -0.04%
==========================================
Files 163 163
Lines 49766 49779 +13
==========================================
- Hits 45423 45419 -4
- Misses 4343 4360 +17
Continue to review full report at Codecov.
|
* 'master' of github.com:pandas-dev/pandas: (188 commits) Separate out _convert_datetime_to_tsobject (pandas-dev#17715) DOC: remove whatsnew note for xref pandas-dev#17131 BUG: Regression in .loc accepting a boolean Index as an indexer (pandas-dev#17738) DEPR: Deprecate cdate_range and merge into bdate_range (pandas-dev#17691) CLN: replace %s syntax with .format in pandas.core: categorical, common, config, config_init (pandas-dev#17735) Fixed the memory usage explanation of categorical in gotchas from O(nm) to O(n+m) (pandas-dev#17736) TST: add backward compat for offset testing for pickles (pandas-dev#17733) remove unused time conversion funcs (pandas-dev#17711) DEPR: Deprecate convert parameter in take (pandas-dev#17352) BUG:Time Grouper bug fix when applied for list groupers (pandas-dev#17587) BUG: Fix some PeriodIndex resampling issues (pandas-dev#16153) BUG: Fix unexpected sort in groupby (pandas-dev#17621) DOC: Fixed typo in documentation for 'pandas.DataFrame.replace' (pandas-dev#17731) BUG: Fix series rename called with str altering name rather index (GH17407) (pandas-dev#17654) DOC: Add examples for MultiIndex.get_locs + cleanups (pandas-dev#17675) Doc improvements for IntervalIndex and Interval (pandas-dev#17714) BUG: DataFrame sort_values and multiple "by" columns fails to order NaT correctly Last of the timezones funcs (pandas-dev#17669) Add missing file to _pyxfiles, delete commented-out (pandas-dev#17712) update imports of DateParseError, remove unused imports from tslib (pandas-dev#17713) ...
Takes the place of #17708, removing funcs instead of moving them.
Had to cpdef
pydt_to_i8
because the func it is replacing in_libs.index
was cdef'd.git diff upstream/master -u -- "*.py" | flake8 --diff