-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
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
Changes from all commits
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 |
---|---|---|
|
@@ -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] | ||
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. is it necessary to test the actual parameters? 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. It is an object, the length is just =0 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. 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() | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.