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

added shebang for python #694

Merged
merged 4 commits into from Nov 13, 2017

Conversation

Projects
None yet
3 participants
@geniusupgrader
Copy link
Contributor

geniusupgrader commented Oct 22, 2017

fixes #690.
The shebang should start on the first line,
and that is why there is no newline between the template and the shebang

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 23, 2017

If you put dashes in the template syntax, I think it should suppress the newlines so the template can be written the obvious way.

{%- block header -%}
add shebang to python, using dashes
better with dashes
@geniusupgrader

This comment has been minimized.

Copy link
Contributor Author

geniusupgrader commented Oct 23, 2017

ok, now with dashes

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 24, 2017

LGTM, could you add a test to watch for this when converting to python?

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 25, 2017

Also, it shouldn't be python but ipython since we rely on ipython as part of the python exporter to handle a number of ipython magics

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 25, 2017

@geniusupgrader If you've allowed edits from maintainers I'm going to push these changes and will merge this. If not… i'll make a PR against yours and will need you to merge it first and then I can merge it here.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 26, 2017

Hmm, I'd actually say that the python shebang was OK. Yes, if you use magics you will get a Python script that can only be run inside IPython, but not all notebooks will use magics, and we don't really encourage using IPython to run scripts when it's not necessary.

I don't feel strongly about this, though.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Oct 31, 2017

Maybe we could detect if a magic is used? It seems like a lot of overhead just to determine whether it should be an ipython or python.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Nov 1, 2017

I'm inclined to just let it be python - even if that won't necessarily work all the time, I think it represents the intent of converting to a .py script. Even run in IPython, you can't guarantee that code from a notebook will work - it might depend on displaying rich output, for instance.

@takluyver takluyver merged commit cf12c9f into jupyter:master Nov 13, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@takluyver takluyver added this to the 5.4 milestone Nov 13, 2017

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