-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Pass through Engine kwargs in ExcelWriter #43445
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
Changes from 11 commits
b18af6f
7c439c8
d214487
e7c2b6e
a4a29ee
44653f9
c94561b
02406a6
4708e3e
fb949f5
916f077
97589ea
8a7e79d
5da5254
f90e7db
f8a8fae
87bc0e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,30 +85,81 @@ def test_write_cells_merge_styled(ext): | |
assert xcell_a2.font == openpyxl_sty_merged | ||
|
||
|
||
@pytest.mark.parametrize("write_only", [True, False]) | ||
def test_kwargs(ext, write_only): | ||
# GH 42286 | ||
# openpyxl doesn't utilize kwargs, only test that supplying a kwarg works | ||
kwargs = {"write_only": write_only} | ||
@pytest.mark.parametrize("iso_dates", [True, False]) | ||
def test_kwargs(ext, iso_dates): | ||
# GH 42286 GH 43445 | ||
kwargs = {"iso_dates": iso_dates} | ||
with tm.ensure_clean(ext) as f: | ||
msg = re.escape("Use of **kwargs is deprecated") | ||
with tm.assert_produces_warning(FutureWarning, match=msg): | ||
with ExcelWriter(f, engine="openpyxl", **kwargs) as writer: | ||
assert writer.book.iso_dates == iso_dates | ||
# ExcelWriter won't allow us to close without writing something | ||
DataFrame().to_excel(writer) | ||
|
||
|
||
@pytest.mark.parametrize("write_only", [True, False]) | ||
def test_engine_kwargs(ext, write_only): | ||
# GH 42286 | ||
# openpyxl doesn't utilize kwargs, only test that supplying a engine_kwarg works | ||
engine_kwargs = {"write_only": write_only} | ||
@pytest.mark.parametrize("iso_dates", [True, False]) | ||
def test_engine_kwargs_write(ext, iso_dates): | ||
# GH 42286 GH 43445 | ||
engine_kwargs = {"iso_dates": iso_dates} | ||
with tm.ensure_clean(ext) as f: | ||
with ExcelWriter(f, engine="openpyxl", engine_kwargs=engine_kwargs) as writer: | ||
assert writer.book.iso_dates == iso_dates | ||
feefladder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# ExcelWriter won't allow us to close without writing something | ||
DataFrame().to_excel(writer) | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"engine_kwargs", [{"data_only": True}, {"keep_vba": True}, {"keep_links": False}] | ||
) | ||
def test_engine_kwargs_append(ext, engine_kwargs): | ||
# GH 43445 | ||
# only read_only=True will give something easy that can be verified (maybe | ||
# keep_links or data_only could also work, but that'd be more complicated) | ||
# arguments are passed through to load_workbook() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think read_only would be sufficient. Need to verify the kwarg gets through. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought testing for all available parameters would be nice, since then we will also be warned when API changes on openpyxl's side... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, read-only will not work here, and give different errors on windows than linux, this is why the previous test failed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm understanding the test correctly, the passing of kwargs through to openpyxl could be entirely removed from pandas and this test would pass. Let me know if you're not sure of a way to improve that (or if my assessment is wrong). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, that's true. That's why I have the other test: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tell me if you want it removed! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the summary, very helpful. Yes, I think this test can be removed as it's redundant with |
||
with tm.ensure_clean(ext) as f: | ||
DataFrame(["hello", "world"]).to_excel(f) | ||
with ExcelWriter( | ||
f, engine="openpyxl", mode="a", engine_kwargs=engine_kwargs | ||
) as writer: | ||
DataFrame(["goodbye", "world"]).to_excel(writer, sheet_name="Sheet2") | ||
|
||
|
||
@pytest.mark.parametrize("data_only,B2", [(True, 0), (False, "=1+1")]) | ||
feefladder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
def test_engine_kwargs_append_keep_links(ext, data_only, B2): | ||
# GH 43445 | ||
# tests whether the keep_links engine_kwarg actually works well for | ||
# openpyxl's load_workbook | ||
# not sure if we have to test for this though, since it also relies a bit on | ||
# functionality of openpyxl | ||
feefladder marked this conversation as resolved.
Show resolved
Hide resolved
|
||
with tm.ensure_clean(ext) as f: | ||
DataFrame(["=1+1"]).to_excel(f) | ||
with ExcelWriter( | ||
f, engine="openpyxl", mode="a", engine_kwargs={"data_only": data_only} | ||
) as writer: | ||
assert writer.sheets["Sheet1"]["B2"].value == B2 | ||
# ExcelWriter needs us to writer something to close properly? | ||
DataFrame().to_excel(writer, sheet_name="Sheet2") | ||
|
||
|
||
def test_engine_kwargs_append_invalid(ext): | ||
# GH 43445 | ||
# test whether an invalid engine kwargs actually raises | ||
with tm.ensure_clean(ext) as f: | ||
DataFrame(["hello", "world"]).to_excel(f) | ||
with pytest.raises( | ||
TypeError, | ||
match=re.escape( | ||
"load_workbook() got an unexpected keyword argument 'apple_banana'" | ||
), | ||
): | ||
with ExcelWriter( | ||
f, engine="openpyxl", mode="a", engine_kwargs={"apple_banana": "fruit"} | ||
) as writer: | ||
# not sure if we have to do something here? | ||
DataFrame(["good"]).to_excel(writer, sheet_name="Sheet2") | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"mode,expected", [("w", ["baz"]), ("a", ["foo", "bar", "baz"])] | ||
) | ||
|
Uh oh!
There was an error while loading. Please reload this page.