Refactor out libwriters, fix references to Timestamp, Timedelta #19413
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19413 +/- ##
==========================================
+ Coverage 91.6% 91.62% +0.02%
==========================================
Files 150 150
Lines 48724 48725 +1
==========================================
+ Hits 44632 44645 +13
+ Misses 4092 4080 -12
Continue to review full report at Codecov.
|
pandas/_libs/writers.pyx
Outdated
| return arr | ||
|
|
||
|
|
||
| def convert_timestamps(ndarray values): |
There was a problem hiding this comment.
rather than just move this, replace it with something like (where its called)
pd.to_datetime(arr, cache=True).to_pydatetime()
pandas/_libs/writers.pyx
Outdated
|
|
||
|
|
||
| @cython.wraparound(False) | ||
| @cython.boundscheck(False) |
There was a problem hiding this comment.
rather than copy this, replace with pd.unique()
pandas/_libs/writers.pyx
Outdated
| bytes | ||
|
|
||
|
|
||
| def sanitize_objects(ndarray[object] values, set na_values, |
pandas/_libs/parsers.pyx
Outdated
|
|
||
| def sanitize_objects(ndarray[object] values, set na_values, | ||
| convert_empty=True): | ||
| cdef: |
pandas/_libs/writers.pyx
Outdated
| def write_csv_rows(list data, ndarray data_index, | ||
| int nlevels, ndarray cols, object writer): | ||
|
|
||
| cdef int N, j, i, ncols |
There was a problem hiding this comment.
can u add some doc strings
| @@ -4621,7 +4622,7 @@ def _get_converter(kind, encoding): | |||
| if kind == 'datetime64': | |||
| return lambda x: np.asarray(x, dtype='M8[ns]') | |||
| elif kind == 'datetime': | |||
There was a problem hiding this comment.
this branch might not even be hit anymore
can u point to where it is exercised?
There was a problem hiding this comment.
It'll be reached if IndexCol.convert is called with an IndexCol of kind == 'datetime'.
There was a problem hiding this comment.
i can read what if says (i wrote this)
what i am asking is which test hits this
this inference should not be true except for an object Index which infers to datetime which we rarely have (as they are always converts)
There was a problem hiding this comment.
Then I have no idea.
There was a problem hiding this comment.
you can remove this kind == 'datetime' branch its not hit by anything
| @@ -4621,7 +4622,7 @@ def _get_converter(kind, encoding): | |||
| if kind == 'datetime64': | |||
| return lambda x: np.asarray(x, dtype='M8[ns]') | |||
| elif kind == 'datetime': | |||
There was a problem hiding this comment.
i can read what if says (i wrote this)
what i am asking is which test hits this
this inference should not be true except for an object Index which infers to datetime which we rarely have (as they are always converts)
| @@ -4621,7 +4622,7 @@ def _get_converter(kind, encoding): | |||
| if kind == 'datetime64': | |||
| return lambda x: np.asarray(x, dtype='M8[ns]') | |||
| elif kind == 'datetime': | |||
There was a problem hiding this comment.
you can remove this kind == 'datetime' branch its not hit by anything
OK. What about the other places where |
|
u might be able to remove them as well |
|
My inclination is to not mess with parts of the code I'm not familiar with. |
This reverts commit 4cbbb73.
|
lgtm. I reverted the last commit, will do in #19475 . ping on green. |
|
Ping |
|
thanks! |
git diff upstream/master -u -- "*.py" | flake8 --diff