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

DataFrameMapper fit doesn't take y argument #58

Closed
vzaretsk opened this issue Jul 8, 2016 · 3 comments
Closed

DataFrameMapper fit doesn't take y argument #58

vzaretsk opened this issue Jul 8, 2016 · 3 comments

Comments

@vzaretsk
Copy link
Contributor

vzaretsk commented Jul 8, 2016

Currently DataFrameMapper fit method doesn't take a y argument. I have a use case that needs this (I'm doing supervised dimensionality reduction) and made a small modification to enable this functionality. If there is interest, I can submit a pull request with these changes. Additionally, it seems that this would eliminate the need for a custom Pipeline class.

@dukebody
Copy link
Collaborator

dukebody commented Jul 9, 2016

Hi @vzaretsk . There has been some discussion regarding this feature already: #54

I feel I may be behaving a bit too defensive here. I believe this could be a good feature to have as long as:

  • Existing code works unchanged. Deprecation plans are acceptable.
  • The DataFramaMapper class doesn't get too complex with a lot larger surface for bugs.

Please submit the PR together with a use case of the change, so we can find out the best way to implement this y-transforming feature.

@vzaretsk
Copy link
Contributor Author

My use case is for the shelter animal outcomes Kaggle competition. Rather than creating over 100 dummy variables for the various cat and dog breeds, I replaced each breed by the average outcome per breed. The y (outcome) information is used during fit to find this average outcome for each breed in the CV fold. Other use cases could be supervised clustering or using LDA to find the direction that best separates the classes.
I submitted the PR. All tests are passing although I will need to make some additional ones if this goes forward. Existing code should work as is.

@dukebody
Copy link
Collaborator

dukebody commented Aug 3, 2016

Resolved by #59 . Thanks a lot @vzaretsk !!!

@dukebody dukebody closed this as completed Aug 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants