dswah / pyGAM

[HELP REQUESTED] Generalized Additive Models in Python
https://pygam.readthedocs.io
Apache License 2.0
862 stars 159 forks source link

Score method #236

Closed JodesL closed 5 years ago

JodesL commented 5 years ago

Hi @dswah, I have added a GAM.score method to the base GAM class following from issue #102. I have also added a simple test for the score method to quickly check that it the score method without crashing and checks that the score which is R^2 is <=1.

Currently it only calculates the R^2 which is the default score in scikit-learn for regression models. I can also add the accuracy score specifically to the LogisticGam class.

I am quite new to github and this whole open source concept, so I would appreciate any feedback! Let me know if the code makes sense or if I am completely off. Thanks!

codecov[bot] commented 5 years ago

Codecov Report

Merging #236 into master will decrease coverage by 0.01%. The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage    95.2%   95.18%   -0.02%     
==========================================
  Files          22       22              
  Lines        3170     3178       +8     
==========================================
+ Hits         3018     3025       +7     
- Misses        152      153       +1
Impacted Files Coverage Δ
pygam/tests/test_GAM_methods.py 100% <100%> (ø) :arrow_up:
pygam/pygam.py 94.79% <80%> (-0.1%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f7b2c75...5b6e587. Read the comment docs.

dswah commented 5 years ago

@JodesL This is amazing!! Thank you!

A couple of initial comments:

I will take a look closer this weekend :) Thanks again for contributing!

dswah commented 5 years ago

Very cool that you included a test :+1:

JodesL commented 5 years ago

@dswah Thanks for replying! I think your option that returns explained deviance for all generic gams is much better. It would then depend on the deviance defined for each distribution which makes sense.

As for LogisticGam, I am thinking I can just override Gam.score() with accuracy instead of R2.

Let me know your thoughts 😀

dswah commented 5 years ago

@JodesL I'm sorry for not replying! I have just started a new job and have been quite busy. Your changes look great. I will merge them this weekend!

dswah commented 5 years ago

@JodesL I would like to change the pyGAM license to allow proprietary use of the library, which means downstream users would NOT need to release the source-code of their application when they use pyGAM.

The current GPLv3 license is quite strict, and the hope is that a more passive license will encourage (proprietary) contributors to keep improving this library.

Please let me know if you are ok with this change. Thanks, Danny

PS: I am considering the following licenses:

Current GPLv3 license for reference. (tldr: any modifications and downstream applications must be made open-source and released with the same license)

JodesL commented 5 years ago

@dswah The changes to the license seems great, would encourage more usage of the package for sure! And no worries I have also been busy recently.