[MRG+1] remove __future__ imports by surgan12 · Pull Request #12791 · scikit-learn/scikit-learn

surgan12 · 2018-12-15T11:34:15Z

benchmarks/bench_20newsgroups.py

eamanu

LGTM. IMO There are some unnecessary white lines.

eamanu · 2018-12-16T01:08:33Z

benchmarks/bench_plot_nmf.py

@@ -6,7 +6,6 @@
 #          Anthony Di Franco (projected gradient, Python and NumPy port)
 # License: BSD 3 clause



unnecessary whiteline?

eamanu · 2018-12-16T01:09:12Z

benchmarks/bench_covertype.py


 """
-from __future__ import division, print_function



unnecessary whiteline?

Thanks Man !

eamanu · 2018-12-16T01:09:50Z

doc/conf.py

@@ -12,7 +12,6 @@
 # All configuration values have a default; values that are commented out
 # serve to show the default.



unnecessary whiteline?

eamanu · 2018-12-16T01:10:43Z

examples/applications/plot_tomography_l1_reconstruction.py

 contributed to fewer projections than the central disk.
 """
-from __future__ import division



unnecessary whiteline?

eamanu · 2018-12-16T01:11:11Z

examples/classification/plot_lda.py

 """

-from __future__ import division



unnecessary (double) whiteline?

eamanu · 2018-12-16T01:12:40Z

sklearn/decomposition/nmf.py



-from __future__ import division, print_function



unnecessary (multiple) whiteline?

eamanu · 2018-12-16T01:13:09Z

sklearn/externals/joblib/parallel.py

 # License: BSD 3 clause

-from __future__ import division



unnecessary whiteline?

eamanu · 2018-12-16T01:13:35Z

sklearn/linear_model/tests/test_theil_sen.py

 # License: BSD 3 clause

-from __future__ import division, print_function, absolute_import



unnecessary whiteline?

surgan12 · 2018-12-16T09:21:33Z

@adrinjalali u there ?

adrinjalali · 2018-12-16T12:35:37Z

@surgan12 please be patient with our responses. Sometimes it takes us a while. It's not ideal, but response latency can be horribly high here. However, I'm looking at the logs. Somehow as you mentioned, removing the division is kinda breaking all the calculations, I need to figure out why.

surgan12 · 2018-12-16T12:39:05Z

Sure !!

…

On Sun, Dec 16, 2018 at 6:06 PM Adrin Jalali ***@***.***> wrote: @surgan12 <https://github.com/surgan12> please be patient with our responses. Sometimes it takes us a while. It's not ideal, but response latency can be horribly high here. However, I'm looking at the logs. Somehow as you mentioned, removing the division is kinda breaking all the calculations, I need to figure out why. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#12791 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AfljYe1p9ZSF2xEOMA_FXWJEX4BAI0jfks5u5j5lgaJpZM4ZUshx> .

sklearn/utils/_random.pyx

sklearn/decomposition/dict_learning.py

adrinjalali · 2018-12-28T12:24:17Z

@surgan12 could you please merge master? The doc build issue is fixed now there.

rth · 2019-01-05T16:32:08Z

So we need to resolve #12906 to make CI green here.

rth · 2019-01-05T16:28:44Z

sklearn/externals/_pilutil.py

 ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
 THE POSSIBILITY OF SUCH DAMAGE.
 """
-from __future__ import division, print_function, absolute_import


Please revert all changes under sklearn/externals: this is vendored code and it's better to keep it in its original state.

rth · 2019-01-05T16:43:30Z

Interestingly "Remove __future__ import" commit in #12928 passes CI fine, also I don't see these failures on master.

could you please merge master? The doc build issue is fixed now there.

[commit] Merge branch 'docs_change'

That commit should have said "Merge branch 'master'" I think. Could you please try again @surgan12 ?

Something along the lines of,

git fetch upstream
git merge upstream/master

where upstream points to scikit-learn/scikit-learn.git not your fork.

adrinjalali · 2019-01-05T17:16:33Z

@rth the tests passed on your PR cause you were not touching examples (I guess). The issue is that the circle-ci on master somehow doesn't actually do a clean complete run. I suspect the tests on master pass because of this line, but not sure.

rth · 2019-01-05T17:23:58Z

circle-ci on master somehow doesn't actually do a clean complete run

As far as I know, only dataset download are cached in .circleci/config.yml here and it spins a new docker container for each run, so assuming all docs get build on master (which seem to be the case) it should do a clean and complete run.

The line you mention in the Makefile was more intended for re-running things locally, it should have no effect on CircleCI.

adrinjalali · 2019-01-16T15:42:47Z

@surgan12 could you do another update from the master? The issue should be solved now.

adrinjalali · 2019-01-31T09:41:10Z

ping @surgan12 , would you like someone else take over this one?

surgan12 · 2019-01-31T10:26:58Z

@adrinjalali sorry , this just slipped out of ma head, I m on it.

adrinjalali · 2019-02-01T10:28:04Z

I think you may have missed these ones:

sklearn/feature_extraction/tests/test_text.py:from __future__ import unicode_literals
sklearn/model_selection/_split.py:from __future__ import print_function
sklearn/model_selection/_split.py:from __future__ import division

adrinjalali

thanks @surgan12 !

adrinjalali · 2019-02-01T11:28:46Z

@rth I think this is finally ready to be merged.

surgan12 · 2019-02-01T11:32:14Z

@adrinjalali , yep finally ready !!

qinhanmin2014 · 2019-02-01T13:04:22Z

sklearn/externals/joblib/_dask.py

@@ -1,4 +1,3 @@
-from __future__ import print_function, division, absolute_import



please double check that there's no redundant blank line (e.g., blank line at the beginning of a file).

rth

I think this is finally ready to be merged.

My comment from #12791 (comment) still needs to be addressed I think.

surgan12 · 2019-02-02T10:21:30Z

@rth on it !!

rth · 2019-02-02T11:26:25Z

sklearn/externals/joblib/_dask.py

@@ -1,5 +1,4 @@
 from __future__ import print_function, division, absolute_import
-


More changes to externals that should be reverted.

If you search for "externals" in the diff on Github that should not contain any files from sklearn/externals.

You can use

git checkout master -- <file_path>

to revert a file to the version from master.

qinhanmin2014 · 2019-02-02T14:05:25Z

thanks @surgan12

surgan12 · 2019-02-02T14:18:09Z

@qinhanmin2014 Was good experience contributing for the first time .

This reverts commit dde62a9.

unimport

c35f8a9

adrinjalali reviewed Dec 15, 2018

View reviewed changes

benchmarks/bench_20newsgroups.py Outdated Show resolved Hide resolved

adrinjalali changed the title ~~unimport~~ remove __future__ imports Dec 15, 2018

complete

727f829

eamanu approved these changes Dec 16, 2018

View reviewed changes

surgan12 added 4 commits December 16, 2018 14:45

Update parallel.py

4f29f14

Update nmf.py

c2ebadd

Update plot_lda.py

718fe8c

Update test_theil_sen.py

5c8ce08

adrinjalali reviewed Dec 16, 2018

View reviewed changes

sklearn/utils/_random.pyx Show resolved Hide resolved

adrinjalali reviewed Dec 16, 2018

View reviewed changes

sklearn/decomposition/dict_learning.py Outdated Show resolved Hide resolved

surgan12 added 2 commits December 16, 2018 19:04

Update _random.pyx

3057eb7

Update dict_learning.py

5389b00

adrinjalali mentioned this pull request Dec 16, 2018

Change cython language_level to 3 #12796

Closed

Update dict_learning.py

02a4a16

adrinjalali mentioned this pull request Dec 16, 2018

[MRG] circle-ci should only run/build plot_* files #12797

Merged

adrinjalali mentioned this pull request Dec 24, 2018

Remove "from __future__ import XXX" #12779

Closed

Merge branch 'docs_change'

679765a

This was referenced Jan 5, 2019

AttributeError: 'PrettyPrinter' object has no attribute '_indent_at_name' #12906

Closed

MAINT Remove unused utils.fixes #12928

Merged

rth reviewed Jan 5, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master'

fd8db5e

master merges

2187c9b

some more fixes

5067333

adrinjalali approved these changes Feb 1, 2019

View reviewed changes

adrinjalali changed the title ~~remove __future__ imports~~ [MRG+1] remove __future__ imports Feb 1, 2019

qinhanmin2014 reviewed Feb 1, 2019

View reviewed changes

rth suggested changes Feb 2, 2019

View reviewed changes

changes reverted

dc59c9d

rth reviewed Feb 2, 2019

View reviewed changes

changes

b5d2497

qinhanmin2014 merged commit 62d2059 into scikit-learn:master Feb 2, 2019

qinhanmin2014 mentioned this pull request Feb 2, 2019

MNT redundant from __future__ import #13079

Merged

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 6, 2019

MNT remove __future__ imports (scikit-learn#12791)

aea504d

thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Feb 7, 2019

MNT remove __future__ imports (scikit-learn#12791)

5727c74

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

MNT remove __future__ imports (scikit-learn#12791)

dde62a9

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

Revert "MNT remove __future__ imports (scikit-learn#12791)"

ceac18a

This reverts commit dde62a9.

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

Revert "MNT remove __future__ imports (scikit-learn#12791)"

322cb78

This reverts commit dde62a9.

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

MNT remove __future__ imports (scikit-learn#12791)

689f48c

		@@ -6,7 +6,6 @@
		# Anthony Di Franco (projected gradient, Python and NumPy port)
		# License: BSD 3 clause

		@@ -12,7 +12,6 @@
		# All configuration values have a default; values that are commented out
		# serve to show the default.

		# License: BSD 3 clause

		from __future__ import division, print_function, absolute_import

		@@ -1,4 +1,3 @@
		from __future__ import print_function, division, absolute_import

		@@ -1,5 +1,4 @@
		from __future__ import print_function, division, absolute_import

Uh oh!

Conversation

surgan12 commented Dec 15, 2018

Uh oh!

Uh oh!

eamanu 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!

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!

surgan12 commented Dec 16, 2018

Uh oh!

adrinjalali commented Dec 16, 2018

Uh oh!

surgan12 commented Dec 16, 2018 via email

Uh oh!

Uh oh!

Uh oh!

adrinjalali commented Dec 28, 2018

Uh oh!

rth commented Jan 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rth commented Jan 5, 2019

Uh oh!

adrinjalali commented Jan 5, 2019

Uh oh!

rth commented Jan 5, 2019

Uh oh!

adrinjalali commented Jan 16, 2019

Uh oh!

adrinjalali commented Jan 31, 2019

Uh oh!

surgan12 commented Jan 31, 2019

Uh oh!

adrinjalali commented Feb 1, 2019

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

adrinjalali commented Feb 1, 2019

Uh oh!

surgan12 commented Feb 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rth left a comment

Choose a reason for hiding this comment

Uh oh!

surgan12 commented Feb 2, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qinhanmin2014 commented Feb 2, 2019

Uh oh!

surgan12 commented Feb 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development