Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't pack every list-like fit param #320

Merged
merged 8 commits into from Jul 30, 2018

Conversation

@TomAugspurger
Copy link
Member

@TomAugspurger TomAugspurger commented Jul 27, 2018

This is an incorrect fix for #319.

Hadcoding list as the only arraylike thing that isn't sent through _indexable isn't right, but I'm not sure what else should be exempted. Mainly because I don't yet understand the purpose of to_indexable.

cc @jcrist do you have any thoughts here?

Closes #319

@TomAugspurger TomAugspurger changed the title [WIP] Don't pack every list-like fit param Don't pack every list-like fit param Jul 30, 2018
@TomAugspurger
Copy link
Member Author

@TomAugspurger TomAugspurger commented Jul 30, 2018

Pushed a new approach, which I think fixes it closer to the source of the problem.

The root cause was improperly assuming that every array-like fit_param should be split like X and y. However, only array-likes with the same number of samples as X and y should be split. Others should just pass through.

Do accomplish this, CVCache now knows the length of X and y. When we unpack a parameter, we split it only if its length matches the length of X and y.

[ci skip]
@TomAugspurger TomAugspurger merged commit 78b9202 into dask:master Jul 30, 2018
0 of 3 checks passed
0 of 3 checks passed
ci/circleci: py27 CircleCI is running your tests
Details
ci/circleci: py36 CircleCI is running your tests
Details
ci/circleci: sklearn_dev CircleCI is running your tests
Details
@TomAugspurger TomAugspurger deleted the TomAugspurger:arraylike-fit-param branch Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

1 participant