Implementation of #129: Allow more tuning of the model hyperparameters.

[sandervh14]

====> Do not merge yet! See below for explanation. <=====

Story Title

#129 Allow more tuning of the model hyperparameters.

Changes made

Altered the constructor of both LogisticRegressionModel and LinearRegressionModel to support override of the default hyperparameters, via kwargs passing.
Documented in the docstring of both models.
See the issue description for more extensive explanation of why one would want this.

** Do not merge this yet with the Development branch. ** Unit tests have not yet been adapted yet. Was a speedy commit of an idea I was playing with in a notebook, when investigating how to further tune the models for a client. I'll take the time next week to adapt the unit tests.

How does the solution address the problem

This PR allows a data scientist to pass argument overrides for the constructor of the scikit-learn model used behind the scenes in Cobra, to try to tune further on a model's performance, if possible vs. Cobra's default settings for the model.

Linked issues

Resolves #129

[sborms]

Looks good Sander! Code is clear to me.

Two remarks:

• We have a get_intercept() function in the model classes - what will the result be if no intercept is fitted? This is a barely used helper function, but better to show a graceful error if there is no intercept to retrieve.
• Agree that for linear models the extra parameters are already taken care of by Cobra, but let's allow for the flexibility. Maybe add a caveat in one of the classes (so at least we have it written down somewhere).

[ZlaTanskY]

This for loop can be replaced by a oneliner by using .update() (see https://www.programiz.com/python-programming/methods/dictionary/update). In that case, default_kwargs best to be renamed to something like model_kwargs.

Suggested change

	default_kwargs = dict(fit_intercept=True)
	for kwarg, val in default_kwargs.items():
	if kwarg not in kwargs:
	kwargs[kwarg] = val
	self.linear = LinearRegression(**kwargs)
	model_kwargs = dict(fit_intercept=True)
	model_kwargs.update(kwargs)
	self.linear = LinearRegression(**model_kwargs)

[sandervh14]

Nice Jano, I was going to commit it straight away with the github interface, but then it isn't applied to LogisitcRegression as well. I'll have a look at it this afternoon and will include Sam's remarks

[ZlaTanskY]

An idea to resolve duplicate code (and documentation to some extent) is to create a BaseModel class from which both the LinearRegression and LogisticRegression class inherit.
But this is of course out of scope for this PR and should be considered in a separate PR.

[sandervh14]

An idea to resolve duplicate code (and documentation to some extent) is to create a BaseModel class from which both the LinearRegression and LogisticRegression class inherit.

I remembered this comment while fixing #126, I also found there was quite some duplication and I had time to do this, so I've done this today with the solution to #126, yippee! :-). For details, see explanation in #128 (comment).

But this is of course out of scope for this PR and should be considered in a separate PR.

I took the liberty to include the superclassing abstraction in #126 anyway, instead of a new issue & PR dedicated to it, since the evaluate() which I was fixing unit tests for, takes up a BIG chunk of the code of both LinearRegression and LogisticRegressionModel.

[sandervh14]

Hey Sam:

• Agree that for linear models the extra parameters are already taken care of by Cobra, but let's allow for the flexibility.

You mean only linear regression right? Do you agree with me that for logistic regression it's certainly a good idea? (most of all for tuning regularization C?)

[sborms]

Hey Sam:

• Agree that for linear models the extra parameters are already taken care of by Cobra, but let's allow for the flexibility.

You mean only linear regression right? Do you agree with me that for logistic regression it's certainly a good idea? (most of all for tuning regularization C?)

Indeed, linear regression. It's definitely a good idea, it's fine to have the flexibility for both logistic & linear regression.

[sandervh14]

Done. Advice: merge #126 first, before this one! I think that'll make merging easier.