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 raise error if model has already been fit #4

Closed
wants to merge 1 commit into from

Conversation

RitwikGupta
Copy link

Related to issue #3

@maciejkula
Copy link
Collaborator

Can I ask why you would like to have this ability?

In general, it is not clear what multiple calls to fit on the same model instance would accomplish that would not be better served by creating a new instance of the model.

In this particular case, if I didn't raise an exception the result would be building an additional set of self.no_trees trees, which is surely not what the user intends, and inconsistent with the no_trees parameter. Much better to raise an exception and prevent the user from shooting herself or himself in the foot.

@RitwikGupta
Copy link
Author

I think this is something more warning-worthy than error causing. It makes more sense to me to print a warning and return out of the method (therefore not building an additional set of trees, I didn't include that in the PR) so the user knows what's going on, but their program doesn't suffer in any way. There are a lot of instances where you'd be working with a lot of data, costing a lot of computation time, and then you go and fit a model. If you fit it twice, accidentally, I don't think that should exit your program, losing all that time you spent re-arranging and cleaning data.

@maciejkula
Copy link
Collaborator

@RitwikGupta this may accomplish what you're looking for: #5

Also, speaking from experience, it always helps to run your data analysis code on a small sample first. That way you discover all the little bugs without wasting a lot of time.

@maciejkula maciejkula closed this Jul 19, 2015
@RitwikGupta RitwikGupta deleted the patch-1 branch July 20, 2015 07:03
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

Successfully merging this pull request may close these issues.

None yet

2 participants