Repository navigation
openml.datasets.get_data() has unpredictable number of return values #402
Description
MWE:
dataset = openml.datasets.get_dataset(40978)
res = dataset.get_data(target=dataset.default_target_attribute)
print(len(res))
dataset = openml.datasets.get_dataset(2)
res = dataset.get_data(target=dataset.default_target_attribute)
print(len(res))
This depends on whether a default target attribute has been specified.
Metadata
Metadata
Assignees
Labels
No labels
Activity
PGijsbers commentedon Apr 15, 2019
The MWE does not work anymore but setting the
dataset.default_target_attributetoNonedoes reproduce the behavior. After chatting with @mfeurer we decided that the desired behavior would be to always return atuple, with the first value to be the data except for a target, and the second to be the target orNone.PGijsbers commentedon Apr 16, 2019
I am not sure I still like this resolution, because of it's asymmetry with
return_categorical_indicatorandreturn_attribute_namesparameters. If they are left unspecified, no additional values are returned. I would prefer that a tuple is always returned iftargetis set (even if it isNone), but not when it is not passed. Perhaps something like this would be nicer?targetparameter, have aseparateparameter, which isFalseby default.If it is
False, it behaves the same way as it currently does when set toNone(i.e. no additional return value).None,strorList[str]), do behavior as above (i.e. always return atuple, possibly with the second value beingNone).PGijsbers commentedon Apr 16, 2019
After discussion with @janvanrijn and @mfeurer we decided that the function should always return all four values.
PGijsbers commentedon Apr 27, 2019
I'm actually going to come back to this. By version 0.9 the default is going to be Pandas Dataframes, which inherently should convey both which attributes are categorical and what their names are. How long do we want to support array output for? Should we consider two functions, e.g. one that always returns it as array (with the
tuple) and the other as dataframe? And only use the array version internally (it's currently still used forrun_model_on_task), until we find time to refactor that too?mfeurer commentedon Apr 29, 2019
Is there a reason to drop this in the near future? I guess it makes it easier for beginners and for people working with software that doesn't support pandas out of the box (such as Auto-sklearn ;))
What would be the upside of this?
How far is pandas support in scikit-learn? Does it reliably work for every model and preprocessor?
PGijsbers commentedon Apr 29, 2019
No idea, honestly.
Mostly not having two functions in one. But after having a closer look I think we can do away with a lot of the array conversions /
iloc-checks that happen in the function (since we enforce the data to be in pandas anyway). Right now we need 80+ lines of code to just load data. I might have a go at refactoring the internals a little after #677 is merged.I suppose not, other than maintenance. I guess it's useful for some packages (: so let's keep it