Skip to content

Fix dataset parsing for categories - #676

Merged
janvanrijn merged 2 commits into
developfrom
fix_dataset_handling
Apr 16, 2019
Merged

Fix dataset parsing for categories#676
janvanrijn merged 2 commits into
developfrom
fix_dataset_handling

Conversation

@mfeurer

Copy link
Copy Markdown
Collaborator

The recent PR #548 changed the behavior of retrieving the categories of a categorical variable by inducing it from the data. In case the data does not contain a category, we loose information about its potential existence. This PR uses the category information provided by the arff file to determine all legal categories of a categorical variable.

@janvanrijnjanvanrijn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me, the unit test drives the point home.

Can you please add a single line comment to specify why this is a 2-liner, so we know in future? Can be merged afterwards.

@codecov-io

codecov-io commented Apr 16, 2019

Copy link
Copy Markdown

Codecov Report

Merging #676 into develop will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@ Coverage Diff @@## develop #676 +/- ##
===========================================
+ Coverage 90.89% 90.95% +0.05% 
===========================================
Files 36 36 Lines 3558 3736 +178 ===========================================
+ Hits 3234 3398 +164 - Misses 324 338 +14
Impacted FilesCoverage Δ
openml/datasets/dataset.py88.55% <100%> (+0.03%)⬆️
openml/runs/run.py90.15% <100%> (ø)⬆️
openml/testing.py95.61% <0%> (+0.28%)⬆️
openml/extensions/sklearn/extension.py90.28% <0%> (+0.36%)⬆️

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 ab8a966...8726b6c. Read the comment docs.

@janvanrijn

Copy link
Copy Markdown
Member

I can attest that all tests are green btw

@mfeurer

Copy link
Copy Markdown
CollaboratorAuthor

Okay, I did so.

@mfeurer
mfeurer requested a review from janvanrijnApril 16, 2019 12:49
@janvanrijn
janvanrijn merged commit 4152f91 into developApr 16, 2019
@janvanrijn
janvanrijn deleted the fix_dataset_handling branch April 16, 2019 14:32
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@mfeurer@codecov-io@janvanrijn