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

SpectralBiclustering.fit doesn't return self #6126

Closed
amueller opened this issue Jan 6, 2016 · 6 comments
Closed

SpectralBiclustering.fit doesn't return self #6126

amueller opened this issue Jan 6, 2016 · 6 comments
Labels

Comments

@amueller
Copy link
Member

@amueller amueller commented Jan 6, 2016

need to check for coclustering, too.

@maniteja123
Copy link
Contributor

@maniteja123 maniteja123 commented Jan 6, 2016

Hi can I work on this issue ? Thanks.

@amueller
Copy link
Member Author

@amueller amueller commented Jan 6, 2016

you sure can :)

@maniteja123
Copy link
Contributor

@maniteja123 maniteja123 commented Jan 6, 2016

Needed to clarify small thing. Basically this fit method of the base class needs to return self. And since it calls the corresponding fit_ method depending on biclustering or coclustering, they needn't be touched.

And my laptop just stopped working. So looked into the issue from my phone. Please let me know if I am missing something here.

Thanks.

@vinayak-mehta
Copy link
Contributor

@vinayak-mehta vinayak-mehta commented Jan 6, 2016

@maniteja123 That should work. But for chaining purposes, I think we might need to return self from SpectralBiclustering.fit too. (same for coclustering)

@maniteja123
Copy link
Contributor

@maniteja123 maniteja123 commented Jan 7, 2016

@vinayak-mehta Thanks for the response. But presently fit calls self._fit. According to this change, fit should be returning self._fit which in turn returns self. But since these are methods of the same class, wouldn't it be better to call self._fit and then return self in fit ?

@amueller
Copy link
Member Author

@amueller amueller commented Jul 15, 2016

If anyone in the sprint wants to work on this, please check the error in #6141 and see if you can figure out where it comes from.

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

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.