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

fixing column masking & typo #53

Merged
merged 1 commit into from
Dec 8, 2015
Merged

Conversation

rcarneva
Copy link
Contributor

@rcarneva rcarneva commented Dec 8, 2015

Two find-and-replaces:

  1. Column masking in the exported pipeline wasn't working correctly for me with SelectKBest on a model since the mask was coming through as a numeric but some of the columns were named. Updated [mask] -> .iloc[:, mask] and it seems to work now, though more testing would be good if it was working as expected on your end.
  2. indeces -> indices

@rhiever rhiever added the bug label Dec 8, 2015
@rhiever
Copy link
Contributor

rhiever commented Dec 8, 2015

Thanks @rcarneva! Both versions work fine on my end, though I believe the .iloc[:, mask]syntax is "more correct." What version of Python/pandas are you running?

I'll merge this in the meantime.

rhiever pushed a commit that referenced this pull request Dec 8, 2015
fixing column masking & typo
@rhiever rhiever merged commit 9a4f3ec into EpistasisLab:master Dec 8, 2015
@rcarneva
Copy link
Contributor Author

rcarneva commented Dec 8, 2015

Python 2.7.10 / pandas 0.17.1

@rhiever
Copy link
Contributor

rhiever commented Dec 8, 2015

Interesting. I'm on Python 3.5 / pandas 0.17.1 and both cases work.

@rcarneva
Copy link
Contributor Author

rcarneva commented Dec 8, 2015

Think I tracked it down. The two features in my training set had (integer) names 1 and 2, and so pandas seemed to have trouble applying the integer mask. Renaming the columns to strings made it work with the original code, but the updated version with iloc seems to work in both situations, so is probably a safer bet.

BTW, really neat idea and project.

@rhiever
Copy link
Contributor

rhiever commented Dec 8, 2015

I had a feeling that was it. Thanks for looking into it!

BTW, really neat idea and project.

Thanks! Happy to have you involved more if you have any ideas or any of the open issues catch your fancy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants