New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG+1]: Fixes for Windows #179

Merged
merged 4 commits into from Dec 12, 2016

Conversation

Projects
None yet
4 participants
@larsoner
Contributor

larsoner commented Dec 9, 2016

Adds AppVeyor and fixes things on Windows (hopefully).

Closes #162.
Closes #176.

- "git clone git://github.com/astropy/ci-helpers.git"
- "powershell ci-helpers/appveyor/install-miniconda.ps1"
- "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%"
- "activate test"

This comment has been minimized.

@larsoner

larsoner Dec 9, 2016

Contributor

@lesteve CI-helpers make it simple to use conda. You just specify some env vars above, git clone, call a few commands and you're done.

This comment has been minimized.

@GaelVaroquaux

GaelVaroquaux Dec 11, 2016

Contributor

Let's go with this for now, but I am not super happy with this implicit dependence. Experience shows that these things evolve, and we have to track their evolution. As a result just to keep a project alive drains resources. Sphinx-gallery is a small project with little resource.

@larsoner larsoner changed the title from FIX: Fixes for Windows to MRG: Fixes for Windows Dec 9, 2016

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 9, 2016

Okay, AppVeyor is happy. Ready for review/merge from my end.

@GaelVaroquaux

This comment has been minimized.

Contributor

GaelVaroquaux commented Dec 9, 2016

Travis is failing.

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 9, 2016

Failures are unrelated to this PR but are underway at #178

@GaelVaroquaux

This comment has been minimized.

Contributor

GaelVaroquaux commented Dec 9, 2016

@larsoner larsoner force-pushed the larsoner:appveyor branch from 5a4ad3f to 2fa6a27 Dec 9, 2016

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 9, 2016

Rebased and AppVeyor is happy

@GaelVaroquaux GaelVaroquaux changed the title from MRG: Fixes for Windows to [MRG+1]: Fixes for Windows Dec 11, 2016

@GaelVaroquaux

This comment has been minimized.

Contributor

GaelVaroquaux commented Dec 11, 2016

+1 for merge on my side

@Titan-C

This comment has been minimized.

Member

Titan-C commented Dec 11, 2016

Ignorant question. Why do temporary files are not deleted by the context manage but by hand?

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 11, 2016

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 11, 2016

@@ -133,6 +133,8 @@ def scan_used_functions(example_file, gallery_conf):
return backrefs
# XXX This figure:: uses a forward slash even on Windows, but the op.join's
# elsewhere will use backslashes...

This comment has been minimized.

@Titan-C

Titan-C Dec 11, 2016

Member

Just for discussion, out of the scope of this PR. But I think paths inside the rst file can all be forward slashes, even on Windows(I can't test it, don't own a windows machine,). Backreferences rely on paths relative to the project home, thus the leading forward slash. I wonder if in backreferences.py we shall join paths with forward slash instead of op.path.join

This comment has been minimized.

@lesteve

lesteve Dec 12, 2016

Contributor

I think this kind of thing should be looked at indeed.

even on Windows(I can't test it, don't own a windows machine,).

Now that we officially support Windows you probably want to have a setup where you can test locally on Windows. I personally have a Windows VM with VirtualBox. At the time you could download the Windows 10 ISO for free from the Microsoft website.

@Titan-C

This comment has been minimized.

Member

Titan-C commented Dec 11, 2016

I would add like we do for travis a build of the gallery after nosetests finish

    - cd doc
    - make html-noplot
    - make html

Otherwise looks fine for me, +1 for the merge. I have not used windows in many years and don't own a machine to test so I'll trust appveyor being happy. Thanks a lot @Eric89GXL

@larsoner larsoner force-pushed the larsoner:appveyor branch 4 times, most recently from c27013f to 149ff10 Dec 12, 2016

@larsoner

This comment has been minimized.

Contributor

larsoner commented Dec 12, 2016

I would add like we do for travis a build of the gallery after nosetests finish

Done

@lesteve lesteve merged commit fdbdf5a into sphinx-gallery:master Dec 12, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@lesteve

This comment has been minimized.

Contributor

lesteve commented Dec 12, 2016

Merged thanks a lot @Eric89GXL!

@Titan-C

This comment has been minimized.

Member

Titan-C commented Dec 12, 2016

PR #182 helped me to catch it. The download buttons that include the zip files with all the examples of the gallery seem to have the wrong link. I also see it on the last build, here. I can not see that on my linux box or travis. And I don't find a reason for a backslash to be missing in the path to those files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment