[work in progress] Dropping Python 2 support #286
Conversation
Codecov Report
@@ Coverage Diff @@
## master #286 +/- ##
==========================================
- Coverage 73.74% 73.57% -0.18%
==========================================
Files 4 4
Lines 617 613 -4
Branches 124 124
==========================================
- Hits 455 451 -4
- Misses 123 124 +1
+ Partials 39 38 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Can you also update the documentation saying that pyglmnet will support only python 3 starting version 1.1? Also you will have to update the documentation saying what's the minimum version of scikit-learn required if people want to use pipelines / cross-validation from scikit-learn.
pyglmnet/base.py
Outdated
| @@ -1,171 +0,0 @@ | |||
| import warnings | |||
There was a problem hiding this comment.
please don't remove this. We don't want a dependency on sklearn. It's a conscious choice.
pyglmnet/pyglmnet.py
Outdated
| @@ -6,7 +6,7 @@ | |||
| from scipy.special import expit | |||
| from scipy.stats import norm | |||
| from .utils import logger, set_log_level | |||
| from .base import BaseEstimator, is_classifier, check_version | |||
| from sklearn.base import BaseEstimator, is_classifier | |||
There was a problem hiding this comment.
see above. We don't want a dependency on sklearn.
pyglmnet/pyglmnet.py
Outdated
| @@ -881,7 +863,7 @@ def score(self, X, y): | |||
|
|
|||
|
|
|||
| class GLMCV(object): | |||
| """Class for estimating regularized generalized linear models (GLM) | |||
| r"""Class for estimating regularized generalized linear models (GLM) | |||
There was a problem hiding this comment.
why do you need this? It's not related to python 2, is it?
There was a problem hiding this comment.
Running tests gave warnings like this. It's unrelated to Python 2.
…west" This reverts commit a80848c.
This reverts commit 5325e2a.
This reverts commit 5d55acf.
… and slowest"" This reverts commit ffb92a7.
This reverts commit 6813f48.
This reverts commit 46c0187.
…om tests and slowest""" This reverts commit f338929.
This reverts commit d53f165.
This reverts commit 5d55acf.
… and slowest"" This reverts commit ffb92a7.
This reverts commit 6813f48.
|
Spent some time reverting the commits related to sklearn - I'm not proud of the resulting git history. Please squash when merging. I will update docs. |
|
Can you also update the Readme and documentation saying we only support python 3 from now on. |
This reverts commit 731f5fd.
|
I think the PR is good to go, can you look over it @jasmainak ? There were some issues with CircleCI, and on the webpage is states "Your project references CircleCI 1.0 or it has no configuration. CircleCI 1.0 and projects without configuration files are no longer supported. You must update your project to use CircleCI 2.0 configuration to continue." I'm not experienced with CircleCI, but I suggest we fix it in another PR if troubles persist. Please squash merge, the history is very ugly. |
|
Done, thanks @tommyod next time I suggest when you are ready you can put [MRG] at the beginning of the title. It's how we usually indicate that it's good to go :) |
This PR closes #285 .
externalsfolder and associated code.base.py, importing from sklearn instead.