Removing dependency on scipy.io.arff #693
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #693 +/- ##
===========================================
- Coverage 90.35% 90.34% -0.02%
===========================================
Files 36 36
Lines 3785 3789 +4
===========================================
+ Hits 3420 3423 +3
- Misses 365 366 +1
Continue to review full report at Codecov.
|
mfeurer
left a comment
mfeurer
left a comment
There was a problem hiding this comment.
Looks good, I think the failures are not actually caused by this pull request, but by some data shuffling on the test server, I'll fix this in a separate PR.
openml/tasks/split.py
Outdated
| split = repetitions[repetition][fold][sample] | ||
|
|
||
| type_ = line[type_idx].decode('utf-8') | ||
| if not isinstance(line[type_idx], str): |
There was a problem hiding this comment.
Is this case actually triggered with liac-arff?
There was a problem hiding this comment.
Hi,
The decode was required for how the file was being read earlier. With liac-arff it reads as str. So yes you are correct. This if-statement is not really triggered.
Removing it.
What does this PR implement/fix? Explain your changes.
_from_arff_file()in tasks/split.py uses liac-arff instead of scipy.io.arff.How should this PR be tested?
Running pytest or checking for unit test status.
Comments.
Some code cleanup will be performed once the unit tests pass.