remove unused time conversion funcs by jbrockmendel · Pull Request #17711 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits4 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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.
- closes #xxxx
- tests added / passed
- passes
git diff upstream/master -u -- "*.py" | flake8 --diff - whatsnew entry
| @@ -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) |
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?
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 cdefd 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 pydt_to_i8 in place of _to_i8 in _libs.index causes a handful of test failures. The main difference AFAICT is that _to_i8 is less reliable, returns some types of arguments unchanged.
So for the time being, this moves _to_i8 to tslib which is at least a more reasonable place for it to hang out. The other removals discussed are still removals.
| @@ -3436,7 +3436,24 @@ def cast_to_nanoseconds(ndarray arr): |
|---|
| return result |
| def pydt_to_i8(object pydt): |
| cdef inline _to_i8(object val): |
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)
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): |
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
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?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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))
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.
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.
ghost pushed a commit to reef-technologies/pandas that referenced this pull request
- '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) ...
alanbato pushed a commit to alanbato/pandas that referenced this pull request
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request
2 participants
