[MRG] Replace absolute imports with relative ones by aditya1702 · Pull Request #13653 · scikit-learn/scikit-learn

aditya1702 · 2019-04-16T03:33:00Z

Reference Issues/PRs

Fixes #13629

What does this implement/fix? Explain your changes.

Replaces absolute imports with relative imports

Any other comments?

aditya1702 · 2019-04-16T04:42:52Z

I will squash commits once this passes

adrinjalali · 2019-04-16T07:59:32Z

you don't need to squash commits, and it's nice if you would avoid a force push. We squash the commits before merge anyway.

jnothman · 2019-04-16T08:27:11Z

Tests are failing @aditya1702. let us know if you need help.

agramfort

Be careful with the diff. You should not change in docstrings for example

agramfort · 2019-04-16T09:32:03Z

sklearn/exceptions.py

    NotFittedError('This LinearSVC instance is not fitted yet'...)

    .. versionchanged:: 0.18
-       Moved from sklearn.utils.validation.


agramfort · 2019-04-16T09:32:34Z

sklearn/exceptions.py


    .. versionchanged:: 0.18
-       Moved from sklearn.base.
+       Moved from .base.


aditya1702 · 2019-04-16T15:39:17Z

you don't need to squash commits, and it's nice if you would avoid a force push. We squash the commits before merge anyway.

@adrinjalali Got it. I dont force push commits :)

agramfort · 2019-04-16T17:14:58Z

sklearn/externals/copy_joblib.sh

 # Note: BSD sed -i needs an argument unders OSX
 # so first renaming to .bak and then deleting backup files
-find joblib -name "*.py" | xargs sed -i.bak "s/from joblib/from sklearn.externals.joblib/"
+find joblib -name "*.py" | xargs sed -i.bak "s/from joblib/from .joblib/"


to revert I think

I think this should be okay, and despite @NicolasHug's comment, the changes in sklearn/externals/joblib are consistent with this

agramfort · 2019-04-16T17:15:46Z

sklearn/externals/joblib/externals/loky/cloudpickle_wrapper.py


 try:
-    from sklearn.externals.joblib.externals.cloudpickle import dumps, loads
+    from ..cloudpickle import dumps, loads


unsure about these ones here @ogrisel @tomMoral

anything joblib related can be ignored since we're unvendoring #13531

agramfort · 2019-04-16T17:16:05Z

sklearn/linear_model/logistic.py

        Note that 'sag' and 'saga' fast convergence is only guaranteed on
        features with approximately the same scale. You can
-        preprocess the data with a scaler from sklearn.preprocessing.
+        .preprocess the data with a scaler from sklearn.preprocessing.


agramfort · 2019-04-16T17:16:10Z

sklearn/linear_model/logistic.py

        Note that 'sag' and 'saga' fast convergence is only guaranteed on
        features with approximately the same scale. You can preprocess the data
-        with a scaler from sklearn.preprocessing.
+        with a scaler from ..preprocessing.


agramfort · 2019-04-16T17:16:19Z

sklearn/model_selection/_split.py

        if not isinstance(cv, Iterable) or isinstance(cv, str):
            raise ValueError("Expected cv as an integer, cross-validation "
-                             "object (from sklearn.model_selection) "
+                             "object (from .) "


agramfort · 2019-04-16T17:16:48Z

sklearn/setup.py

 import os

-from sklearn._build_utils import maybe_cythonize_extensions
+from _build_utils import maybe_cythonize_extensions


agramfort · 2019-04-16T17:17:02Z

sklearn/utils/estimator_checks.py

    Additional tests for classifiers, regressors, clustering or transformers
    will be run if the Estimator class inherits from the corresponding mixin
-    from sklearn.base.
+    from ..base.


aditya1702 · 2019-04-17T00:47:24Z

cc @agramfort @jnothman @NicolasHug

NicolasHug · 2019-04-17T11:06:44Z

That looks good for the most part, but please revert the changes related to sklearn.external.joblib. Sorry, I should have pointed that earlier in the issue. You should be able to do this simply with

git checkout master -- sklearn/externals/joblib

(also please merge master to sort out the merge conflict with sklearn/metrics/cluster/bicluster.py)

jnothman · 2019-04-17T04:40:31Z

sklearn/externals/copy_joblib.sh

 # Note: BSD sed -i needs an argument unders OSX
 # so first renaming to .bak and then deleting backup files
-find joblib -name "*.py" | xargs sed -i.bak "s/from joblib/from sklearn.externals.joblib/"
+find joblib -name "*.py" | xargs sed -i.bak "s/from joblib/from .joblib/"


I think this should be okay, and despite @NicolasHug's comment, the changes in sklearn/externals/joblib are consistent with this

aditya1702 · 2019-04-17T16:29:30Z

@NicolasHug Resolved the conflicts

NicolasHug · 2019-04-17T16:45:25Z

Thanks @aditya1702 !

)

…learn#13653)" This reverts commit 348aa65.

)

aditya1702 added 2 commits April 15, 2019 23:31

Replace absolute imports with relative ones

e8289e3

fix import error

49e09ca

aditya1702 changed the title ~~[MRG] Replace absolute imports with relative ones~~ [WIP] Replace absolute imports with relative ones Apr 16, 2019

add a . before import

1be879d

agramfort reviewed Apr 16, 2019

View reviewed changes

aditya1702 added 2 commits April 16, 2019 11:53

revert changes in docstrings

fdb5557

remove . before _build_utils

2f6ec3d

agramfort reviewed Apr 16, 2019

View reviewed changes

remove relative imports from docstrings - 2

2d36952

aditya1702 changed the title ~~[WIP] Replace absolute imports with relative ones~~ [MRG] Replace absolute imports with relative ones Apr 17, 2019

jnothman approved these changes Apr 17, 2019

View reviewed changes

aditya1702 added 3 commits April 17, 2019 11:43

revert import changes in joblib folder

864062c

revert changes in copy_joblib.sh

4560105

Merge branch 'master' into fix-absolute-imports

f5752b6

agramfort approved these changes Apr 17, 2019

View reviewed changes

NicolasHug merged commit 301076e into scikit-learn:master Apr 17, 2019

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Apr 25, 2019

MAINT Replace absolute imports with relative imports (scikit-learn#13653

6b29d38

)

aditya1702 deleted the fix-absolute-imports branch April 25, 2019 18:03

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

MAINT Replace absolute imports with relative imports (scikit-learn#13653

348aa65

)

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "MAINT Replace absolute imports with relative imports (scikit-…

e5c12c8

…learn#13653)" This reverts commit 348aa65.

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "MAINT Replace absolute imports with relative imports (scikit-…

80b4e71

…learn#13653)" This reverts commit 348aa65.

koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019

MAINT Replace absolute imports with relative imports (scikit-learn#13653

8f53a66

)

Uh oh!

Conversation

aditya1702 commented Apr 16, 2019

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

aditya1702 commented Apr 16, 2019

Uh oh!

adrinjalali commented Apr 16, 2019

Uh oh!

jnothman commented Apr 16, 2019

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aditya1702 commented Apr 16, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aditya1702 commented Apr 17, 2019

Uh oh!

NicolasHug commented Apr 17, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aditya1702 commented Apr 17, 2019

Uh oh!

NicolasHug commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants