BUG: unsupported type Interval when writing dataframe to excel #19244
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19244 +/- ##
=======================================
Coverage 91.65% 91.65%
=======================================
Files 150 150
Lines 48724 48724
=======================================
Hits 44658 44658
Misses 4066 4066
Continue to review full report at Codecov.
|
doc/source/whatsnew/v0.23.0.txt
Outdated
| @@ -478,3 +478,4 @@ Other | |||
| ^^^^^ | |||
|
|
|||
| - Improved error message when attempting to use a Python keyword as an identifier in a ``numexpr`` backed query (:issue:`18221`) | |||
| - Bug in :func:`DataFrame.to_excel` where an exception was raised indicating unsupported type when writing a data frame containing a column of Interval type (:issue:`19242`) | |||
There was a problem hiding this comment.
move to IO. just say unsupported type
pandas/io/excel.py
Outdated
| @@ -788,6 +789,8 @@ def _conv_value(val): | |||
| val = "{val}".format(val=val) | |||
| elif is_list_like(val): | |||
There was a problem hiding this comment.
this routine needs some re-factoring, something like
# doc-string here
if is_integer(val):
val = int(val)
elif is_float(val):
val = float(val)
elif is_bool(val):
val = bool(val)
elif isinstance(val, (datetime, timedelta)):
pass
else:
# comment here
val = str(val)
return val
with some additional tests
There was a problem hiding this comment.
This raises another issue. I take your meaning here since datetime and date are treated later on in write_cells, but timedelta is not explicitly dealt with. Turns out that writing timedelta is not supported when exporting .xls. Looks like this is an old issue (#9155).
I suggest making the same conversion that xlsxwriter makes in the appropriate write_cells.
There was a problem hiding this comment.
@cbertinato yes this function actually could be moved to the top-level.
jreback
added
Dtype Conversions
|
Hello @cbertinato! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on January 27, 2018 at 02:19 Hours UTC |
doc/source/whatsnew/v0.23.0.txt
Outdated
| @@ -418,6 +418,8 @@ I/O | |||
| - Bug in :func:`read_sas` where a file with 0 variables gave an ``AttributeError`` incorrectly. Now it gives an ``EmptyDataError`` (:issue:`18184`) | |||
| - Bug in :func:`DataFrame.to_latex()` where pairs of braces meant to serve as invisible placeholders were escaped (:issue:`18667`) | |||
| - Bug in :func:`read_json` where large numeric values were causing an ``OverflowError`` (:issue:`18842`) | |||
| - Type ``Interval`` now supported in :func:`DataFrame.to_excel` for all Excel file types (:issue:`19242`) | |||
There was a problem hiding this comment.
use :class:`Interval` , and :class`Timdelta` (this is more user friendly as you almost always have a Timedelta)
pandas/io/excel.py
Outdated
| @@ -21,7 +21,7 @@ | |||
| from pandas.io.common import (_is_url, _urlopen, _validate_header_arg, | |||
| get_filepath_or_buffer, _NA_VALUES, | |||
| _stringify_path) | |||
| from pandas.core.indexes.period import Period | |||
| # from pandas.core.indexes.period import Period | |||
There was a problem hiding this comment.
Import no longer necessary because the elif on line 787 was folded into the else. I'll remove it instead of commenting.
pandas/io/excel.py
Outdated
| @@ -1461,6 +1464,12 @@ def write_cells(self, cells, sheet_name=None, startrow=0, startcol=0, | |||
| elif isinstance(cell.val, date): | |||
| num_format_str = self.date_format | |||
|
|
|||
| if isinstance(cell.val, timedelta): | |||
| delta = cell.val | |||
There was a problem hiding this comment.
just use delta.total_seconds()
pandas/tests/io/test_excel.py
Outdated
| @@ -1440,6 +1440,62 @@ def test_excel_date_datetime_format(self): | |||
| # to use df_expected to check the result | |||
| tm.assert_frame_equal(rs2, df_expected) | |||
|
|
|||
| # GH19242 - test writing Interval without labels | |||
There was a problem hiding this comment.
comments go inside the function
pandas/tests/io/test_excel.py
Outdated
| rand = np.random.randint(-10, 10, size=(20, 2)) | ||
| frame = DataFrame(rand) | ||
| intervals = pd.cut(frame[0], 10) | ||
| frame['new'] = intervals |
There was a problem hiding this comment.
can you make this a little less verbose
use expected for the comparison variable
pandas/io/excel.py
Outdated
| """ Convert numpy types to Python types for the Excel writers. | ||
| :obj:`datetime` and :obj:`date` formatting must be handled in the | ||
| writer. :obj:`str` representation is returned for all other types. | ||
| """ |
There was a problem hiding this comment.
Write an expanded docstring with Parameters and Returns section if you're going to do this.
|
small doc string comments, and pls rebase. ping on green. |
cbertinato
force-pushed
the
issue-19242
branch
2 times, most recently
from
1464ec0 to
9b89a6e
Compare
|
Looks like the AppVeyor failures are in the parquet tests. Are those coming from the rebase? |
|
can you rebase. |
|
can you rebase. and ping on green. |
cbertinato
force-pushed
the
issue-19242
branch
from
9b89a6e to
8859dc5
Compare
|
All green! |
|
thanks @cbertinato ! keep em coming! |
closes to_excel with engine='xlwt' doesn't support pandas.tslib.Timedelta #9155