Skip to content

Fix 779 - #787

Merged
mfeurer merged 7 commits into
developfrom
fix_779
Sep 30, 2019
Merged

Fix 779#787
mfeurer merged 7 commits into
developfrom
fix_779

Conversation

@PGijsbers

@PGijsbers PGijsbers commented Sep 27, 2019

Copy link
Copy Markdown
Collaborator

Fixes #779. If the pickle file is corrupt, data will be loaded from arff instead.

Also:

  • refactor some of the data loading flow
  • prevent _convert_array_format from issuing warnings on np.ndarray -> np.ndarray and pd.DataFrame -> pd.DataFrame conversions.

@codecov-io

codecov-io commented Sep 27, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #787 into develop will decrease coverage by 0.08%.
The diff coverage is 72.72%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #787      +/-   ##
===========================================
- Coverage    87.74%   87.66%   -0.09%     
===========================================
  Files           36       36              
  Lines         4187     4200      +13     
===========================================
+ Hits          3674     3682       +8     
- Misses         513      518       +5
Impacted Files Coverage Δ
openml/datasets/dataset.py 83.52% <72.72%> (-0.55%) ⬇️
openml/_api_calls.py 83.11% <0%> (-1.3%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dcac17e...dcacacf. Read the comment docs.

@PGijsbers
PGijsbers requested a review from mfeurer September 27, 2019 13:48
mfeurer
mfeurer reviewed Sep 27, 2019
View reviewed changes

@mfeurer mfeurer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks very good, thanks a lot! I added a few comments throughout.

Comment thread openml/datasets/dataset.py Outdated
Comment thread openml/datasets/dataset.py Outdated
Comment thread openml/datasets/dataset.py Outdated
Comment thread openml/datasets/dataset.py
Comment thread openml/datasets/dataset.py Outdated
Comment thread tests/test_datasets/test_dataset.py Outdated
@PGijsbers

PGijsbers commented Sep 27, 2019 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I reverted the change to shortcut the conversion warning.
The warning is now written to log instead of raised.
The problem is that it is also called in dataset#648 to convert the y data from pd.Series to pd.DataFrame, which it wouldn't do. I did not think swallowing a pd.Series to pd.DataFrame conversion request was good, so I opted to rather swallow none at all.

@mfeurer
mfeurer merged commit de0335c into develop Sep 30, 2019
@mfeurer
mfeurer deleted the fix_779 branch September 30, 2019 14:43
@PGijsbers PGijsbers mentioned this pull request Oct 14, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mfeurer mfeurer mfeurer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

openml-python does not recognize corrupt dataset.pkl.py3 file

3 participants

@PGijsbers @codecov-io @mfeurer