[MRG+2] Enh: Reorganize build files #6396

nelson-liu · 2016-02-18T15:08:41Z

This PR addresses issue #6359 and gets rid of some redundant code in_build_utils/init.py

arthurmensch · 2016-02-18T15:40:15Z

build_tools/windows/windows_testing_downloader.ps1

+    return
+}
+
+main


This file content was not changed isn't it ?

Nope, nothing besides the rename

Hmmm interesting why this didn't show up as a git move :/ Did you do the remove / add in separate commits? Would you mind squashing your commits please?

Ah no you seem to have done that in a single commit only - (nelson-liu@231e95a)

Maybe the file mode was changed? (git config core.fileMode false)

arthurmensch · 2016-02-18T15:45:11Z

LGTM, let's wait for appveyor to finish. @rvraghav93 this could be merged along with PR #6254, I guess the easiest way is for you to rebase on this one, your call :)

raghavrv · 2016-02-18T16:00:44Z

This LGTM too!

@arthurmensch Whops :/ Sorry that one is merged now :@ But I think since this PR does not change the cythonize.py, the rebase is trivial...

@nelson-liu could you rebase? Git should automatically resolve the conflicts I feel...

raghavrv · 2016-02-18T16:05:05Z

Other than that, you can prefix your PR with MRG+2 and wait for final review + merge... Thanks for the PR!

raghavrv · 2016-02-18T16:08:26Z

sklearn/_build_utils/__init__.py

-try:
-    WindowsError
-except NameError:
-    WindowsError = None


~~Do we need this? (Sorry if I missed something, but on a unix system, why would a WindowsError be raised?)~~ Totally missed that this was in the removed diff

@ogrisel

You can clean up the useless WindowsError and cython.dat in sklearn/_build_utils/init.py while you are at it.

fix: change path for building doc on circleci enh: removed useless WindowError and cythonize.dat in _build_utils init

nelson-liu · 2016-02-18T17:41:44Z

I've rebased and squashed the commits, let me know if there's anything else you'd like me to do

raghavrv · 2016-02-18T17:44:18Z

You have to address this comment (clean up the WindowsError like @ogrisel suggested)

nelson-liu · 2016-02-18T17:52:11Z

@rvraghav93 it looks removed to me?

raghavrv · 2016-02-18T18:07:28Z

Oh yea! Sorry! Scratch that. We'll wait for a day or two for comments from anyone and merge if there is no objection!

GaelVaroquaux · 2016-02-19T10:41:25Z

LGTM. Thank you very much!

[MRG+2] Enh: Reorganize build files

arthurmensch reviewed Feb 18, 2016
View reviewed changes

raghavrv reviewed Feb 18, 2016
View reviewed changes

nelson-liu changed the title ~~Enh: Reorganize build files~~ [MRG+2] Enh: Reorganize build files Feb 18, 2016

nelson-liu force-pushed the reorganize_build_files branch from e937b6f to c393d39 Compare February 18, 2016 17:36

enh: reorganize build files

4b7a0e0

fix: change path for building doc on circleci enh: removed useless WindowError and cythonize.dat in _build_utils init

nelson-liu force-pushed the reorganize_build_files branch from c393d39 to 4b7a0e0 Compare February 18, 2016 17:41

GaelVaroquaux added a commit that referenced this pull request Feb 19, 2016

Merge pull request #6396 from nelson-liu/reorganize_build_files

2141869

[MRG+2] Enh: Reorganize build files

GaelVaroquaux merged commit 2141869 into scikit-learn:master Feb 19, 2016

nelson-liu mentioned this pull request Feb 21, 2016

Make cython does not work #6413

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MRG+2] Enh: Reorganize build files #6396

[MRG+2] Enh: Reorganize build files #6396

+                  return
+              }
+              main

[MRG+2] Enh: Reorganize build files #6396

[MRG+2] Enh: Reorganize build files #6396

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment