Allow choosing sheets when constructing a Table #2076
Conversation
|
This looks like a change that could benefit from the third checkbox (documentation). |
Codecov Report
@@ Coverage Diff @@
## master #2076 +/- ##
==========================================
- Coverage 69.93% 68.96% -0.98%
==========================================
Files 315 315
Lines 53979 53981 +2
==========================================
- Hits 37752 37226 -526
- Misses 16227 16755 +528Continue to review full report at Codecov.
|
Orange/tests/test_xlsx_reader.py
Outdated
| @@ -48,6 +48,11 @@ def test_named_sheet(self): | |||
| self.assertEqual(len(table.domain.attributes), 4) | |||
| self.assertEqual(table.name, 'header_0_sheet-my_sheet') | |||
|
|
|||
| def test_named_sheet_table(self): | |||
| table = Table(get_dataset("header_0_sheet.xlsx"), sheet="my_sheet") | |||
There was a problem hiding this comment.
Can we avoid polluting the constructor? I.e. encouraging the use of Table.from_file()?
There was a problem hiding this comment.
Constructor did not change at all with this PR, here it is just used as usual.
There was a problem hiding this comment.
Usage implies sheet is an argument accepted in the constructor. Any future re-design will need to take this instantaneous whim into consideration. Our Table constructor doesn't have a docstring. This is not the usual.
There was a problem hiding this comment.
True, that is exactly why I thought to include it here like this: that we would know what we break. :) But here I think it would actually be better to modify the constructor not to pass kwargs to from_file so that this would not even be allowed. What do you think?
There was a problem hiding this comment.
Indeed, I don't at all mind not passing kwargs to from_file method.
Includes