Repository navigation
Datasets cached pre-#548 can no longer be used for run_model_on_task #646
Description
It looks like pre-#548 pickled data was in np.array format, where now the default is pandas.dataframe. When I now try to use run_model_on_task for which I still have a cached dataset with np.array as data instead of pd.DataFrame, this line is called with (data, dataset_format=="array", [some list of attribute names). This raises an error because as far as I can tell _convert_array_format assumes that the input data is pd.DataFrame if specified dataformat is "array", which makes this line raise an error because np.array does not have an attribute columns.
The fix seems easy enough, just check if data is already of the preferred type, e.g. start the function with
def _convert_array_format(data, array_format, attribute_names):
if array_format == "array" and not scipy.sparse.issparse(data):
if isinstance(data, np.ndarray):
return data
...
Does this make sense? Shall I set up a PR?
@glemaitre @mfeurer
Metadata
Metadata
Assignees
Labels
No labels
Activity
mfeurer commentedon Mar 18, 2019
Thanks for raising this issue. I would suggest that we use an aggressive updating strategy and re-download the data in case the pickle file cannot be read. What do you think?
PGijsbers commentedon Mar 18, 2019
Not sure, on the one hand I am currently performing some runs with this hack and it seems pretty stable (so far on 90+ different tasks already done). It's not that the pickle file can't be read. It can be and seems to be done correctly. It is just after loading the data,
run_model_on_taskwants to have it innp.ndarrayformat and in the conversion assumes it is not yet innp.ndarrayformat which causes the problem.So I don't think there is anything inherently wrong/breaking with not invalidating peoples cache, which makes the 'hack' ok.
On the other hand, we probably don't want to worry about this scenario anymore later. And writing code that locally changes the stored data (I think it should be possible without redownloading? I didn't look too closely) probably takes more time than just forcing a cache refresh.
mfeurer commentedon Mar 19, 2019
The reason why I proposed this rather aggressive strategy is that new features (such as string support in datasets) will only be available if the data is stored as a pandas array.
I also wouldn't like to reformat the data locally and therefore suggest to either download it again or re-read the arff file to create the pickle of a pandas dataframe.
Would you like to work on this @PGijsbers ?
PGijsbers commentedon Mar 19, 2019
That makes sense. I can work on this.
Fix backwards compatibility #646. Reprocess ARFF file if outdated dat…
Fix backwards compatibility #646. (#654)
mfeurer commentedon Apr 1, 2019
Fixed via #646.