Skip to content

ENH Pass kwds from ExcelFile ctr to xlrd.open_workbook. For example, thi... #4439

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pandas/io/excel.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ def __init__(self, path_or_buf, kind=None, **kwds):
self.tmpfile = None

if isinstance(path_or_buf, compat.string_types):
self.book = xlrd.open_workbook(path_or_buf)
self.book = xlrd.open_workbook(path_or_buf, **kwds)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be named kwds? or can open_workbook deal with any?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xlrd.open_workbook accepts a number of parameters: http://www.lexicon.net/sjmachin/xlrd.html#xlrd.open_workbook-function

That being said I saw the kwd argument in the ExcelFile constructor. Currently that kwd is not used which means if the user tries to pass any additional parameters, or misspells them, they are silently dropped.

This change not only allows passing more arguments down to xlrd but also adds an additional runtime check.

else:
data = path_or_buf.read()
self.book = xlrd.open_workbook(file_contents=data)
self.book = xlrd.open_workbook(file_contents=data, **kwds)

def parse(self, sheetname, header=0, skiprows=None, skip_footer=0,
index_col=None, parse_cols=None, parse_dates=False,
Expand Down
Binary file added pandas/io/tests/data/merged.xls
Binary file not shown.
18 changes: 18 additions & 0 deletions pandas/io/tests/test_excel.py
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,24 @@ def test_excel_table(self):
tm.assert_frame_equal(df4, df.ix[:-1])
tm.assert_frame_equal(df4, df5)

def test_excel_read_merged_cells(self):
_skip_if_no_xlrd()

pth = os.path.join(self.dirpath, 'merged.xls')
xls = ExcelFile(pth, formatting_info=True)
book = xls.book
sheet = book.sheet_by_index(0)
merged_cells = sheet.merged_cells

self.assertEquals(len(merged_cells), 1)
rlo, rhi, clo, chi = merged_cells[0]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it necessary to test the actual parameters? merged_cells won't even be on the object if you don't pass formatting_info=True

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an object, the length is just =0

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's close this PR. Just create a new one that tests that you can pass an xlrd workbook to the xlrd reader and that it all works. Then none of these tests will be necessary.


self.assertEquals(rlo, 1)
self.assertEquals(rhi, 1+1)

self.assertEquals(clo, 0)
self.assertEquals(chi, 1+1)

def test_excel_read_buffer(self):
_skip_if_no_xlrd()
_skip_if_no_openpyxl()
Expand Down