ENH: adding autofilter when writing to excel (pandas-dev#61194) #62994
Conversation
WillAyd
left a comment
WillAyd
left a comment
There was a problem hiding this comment.
Small comment but overall looks like a really nice PR. @rhshadrach any thoughts?
pandas/tests/io/excel/test_style.py
Outdated
| ws = wb.active | ||
|
|
||
| assert ws.auto_filter.ref is not None | ||
| print(ws.auto_filter.ref) |
There was a problem hiding this comment.
Can you remove the print statement?
There was a problem hiding this comment.
Print statement removed - thanks for checking!
WillAyd
left a comment
WillAyd
left a comment
There was a problem hiding this comment.
lgtm - let's see what others think
feat: Throw an error when autofilter set for 'odf' engine. feat: Shows warning when autofilter is set together with merge_cells. chore: Added tests for index=True, startrow/startcol, multindex with merge_cells.
|
@rhshadrach I believe the issues I got with the checks is not related to my changes. I'm waiting for #63107 to get all the checks green. Besides that the code changes are ready for your review. I hope I was able to address your feedback. List of changes: fix: Ensure that index size is considered when setting autofilter range. |
rhshadrach
left a comment
rhshadrach
left a comment
There was a problem hiding this comment.
Previous request was for a DataFrame with MultiIndex columns:
columns = MultiIndex(
levels=[["x", "y"], ["w", "t"]],
codes=[[0, 0, 1], [0, 1, 0]],
)
df = pd.DataFrame([[1, 2, 3], [4, 5, 6]], columns=columns)but agreed as you've found a MultiIndex index also gives problems. I think we should raise when either columns or index is a MultiIndex, autofilter=True, and merge_cells=True.
pandas/io/formats/excel.py
Outdated
| "Excel filters merged cells by showing only the first row." | ||
| "'autofiler' and 'merge_cells' should not " | ||
| "be used simultaneously.", |
There was a problem hiding this comment.
Because of blanks, the autofilter here will produce what I consider invalid results. As such, I think we should raise rather than warn.
There was a problem hiding this comment.
Done - exception is raised when autofilter=True, and merge_cells=True
pandas/tests/io/excel/test_style.py
Outdated
| DataFrame().to_excel(writer, autofilter=True, index=False) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("engine", ["xlsxwriter", "openpyxl"]) |
There was a problem hiding this comment.
For each test, can you test all engines with something like:
if engine in [...]:
with pytest.raises(...):
...
returnThis makes the test suite more robust when adding a new engine. You might find it easier to move these over to test_writers with TestExcelWriter and the fixtures provided there. But no problem if these stay here.
There was a problem hiding this comment.
Done - tests refectored and moved to test_writers module
fix: Handle multindex columns offset feat: If autofilter is set together with merge_cells, raise an exception
|
@rhshadrach feedback addressed. The major code change is related to handling MultiIndex columns - autofiler is added at the bottom row and end range has to be shifted to handle extra blank row added between headers and data. |
rhshadrach
left a comment
rhshadrach
left a comment
There was a problem hiding this comment.
Looks great! One last nit and we're good here.
pandas/io/formats/excel.py
Outdated
| It should have signature css_declarations string -> excel style. | ||
| This is only called for body cells. | ||
| autofilter : bool, default False | ||
| If True, add automatic filters to all columns |
There was a problem hiding this comment.
nit
| If True, add automatic filters to all columns | |
| If True, add automatic filters to all columns. |
|
Thanks @jarent-nvidia - fantastic work here. |
3 participants
doc/source/whatsnew/v3.0.0.rstfile if fixing a bug or adding a new feature.Description of the change:
Added boolean 'autofilter' optional argument to 'DataFrame.to_excel' function to add automatic filters to all columns in the exported excel files. As discussed with @WillAyd starting simple by allowing to add filters only to all or none columns.
New option supported by 'xlsxwriter' and 'openpyxl' engines.