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

Multi variable linear regression #5

Merged
merged 7 commits into from
Dec 10, 2017
Merged

Multi variable linear regression #5

merged 7 commits into from
Dec 10, 2017

Conversation

saroele
Copy link
Member

@saroele saroele commented Dec 7, 2017

I ported the regression to our new library. Main change: improved the handling of column names in the passed dataframe and now it is fully tested.

@saroele
Copy link
Member Author

saroele commented Dec 7, 2017

Maybe @JrtPec can have a quick look and merge?
Ah, we also have to write documentation, fancy the plots and provide an example notebook...

@coveralls
Copy link

Coverage Status

Coverage increased (+20.8%) to 90.244% when pulling 16d9b86 on issue3_linreg into 3c349bc on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+20.8%) to 90.244% when pulling 16d9b86 on issue3_linreg into 3c349bc on master.

@saroele saroele merged commit cc56b03 into master Dec 10, 2017
@JrtPec
Copy link
Member

JrtPec commented Dec 11, 2017

I have looked at the code, it is mostly what we've already seen before so its fine.

I have only 1 style issue: the use of **kwargs in the __init__ method. It makes the signature hard to read (you need to check the docs to see what the options are), and any IDE's prediction will not show the optional arguments.
So I propose to explicitly ask for arguments in the __init__ method.

You'll also need to deliver the material to add the analysis to the website though. We will go through that process next meeting ;-)

@saroele
Copy link
Member Author

saroele commented Dec 12, 2017

Thanks Jan. I had merged it already because of some code I'm running at 3E that needed the regression in the last pypi release.

Good point on kwargs in init. I'll change that.
I created an example notebook, but it's a bit silly without weather data (regression of one gas consumption on another :-/ ) So next monday we have to add a weather dataset for 2016, update the example and prepare website material. Nice agenda!

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

3 participants