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

Fix for when sys.argv is used in examples #252

Merged
merged 2 commits into from Jul 20, 2017

Conversation

Projects
None yet
2 participants
@lesteve
Contributor

lesteve commented Jun 15, 2017

Fix #251.

The fix is not the nicest one (maybe changing sys.argv inside a try clause and changing it back in except would make it a bit better) but it works ...

@lesteve lesteve force-pushed the lesteve:fix-sys-argv branch from 24dd2b6 to 653d2cd Jul 20, 2017

@lesteve

This comment has been minimized.

Contributor

lesteve commented Jul 20, 2017

CHANGES.rst Outdated
@@ -4,6 +4,7 @@ Change Log
git master
----------
<<<<<<< 5eb83a7c61a79656f24e61f5c90c631082265359

This comment has been minimized.

@Titan-C

Titan-C Jul 20, 2017

Member

Left over from rebase?

This comment has been minimized.

@lesteve

lesteve Jul 20, 2017

Contributor

fixed.

@@ -0,0 +1,14 @@
"""
Using ``sys.argv`` in examples

This comment has been minimized.

@Titan-C

Titan-C Jul 20, 2017

Member

This needs to be an RST heading

This comment has been minimized.

@lesteve

lesteve Jul 20, 2017

Contributor

fixed

argv_orig = sys.argv[:]
if block_vars['execute_script']:
print('Executing file %s' % src_file)

This comment has been minimized.

@Titan-C

Titan-C Jul 20, 2017

Member

We currently use the Sphinx logger not the print statement. The execution of the file is stated in generate_dir_rst and not here anymore.

This comment has been minimized.

@lesteve

lesteve Jul 20, 2017

Contributor

Removed the print statement. I think that is what you were aiming at.

argv_orig = sys.argv[:]
if block_vars['execute_script']:
print('Executing file %s' % src_file)
sys.argv[0] = src_file

This comment has been minimized.

@Titan-C

Titan-C Jul 20, 2017

Member

Is this necessary? sys.argv[0] corresponds to the executing binary, most probably it would be python as example scripts are not executable on their own (there is no shebang). I wouldn't like to have this hint to the src_file, because people might start abusing it. We already don't provide the __file__ variable because of incompatibility with the Jupyter notebooks.

This comment has been minimized.

@lesteve

lesteve Jul 20, 2017

Contributor

Otherwise sys.argv[0] is sphinx-build leading to a very confusing error message like the one shown in #251 (comment).

@Titan-C

There are some issues pending. Maybe the try block is not necessary, changing the sys.argv value can be enough.

@lesteve lesteve force-pushed the lesteve:fix-sys-argv branch from 653d2cd to 5630c5c Jul 20, 2017

@Titan-C Titan-C merged commit e91ce43 into sphinx-gallery:master Jul 20, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Titan-C

This comment has been minimized.

Member

Titan-C commented Jul 20, 2017

Thanks, merging

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