Skip to content
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

Download as .ipynb returns the associated python file #118

Closed
Fubukimaru opened this issue Oct 30, 2018 · 12 comments
Closed

Download as .ipynb returns the associated python file #118

Fubukimaru opened this issue Oct 30, 2018 · 12 comments
Assignees

Comments

@Fubukimaru
Copy link

Hello,

When I'm on a notebook and I want to make a copy as .ipynb, I go to Download as > .ipynb, however what I get is the .py instead. Would it be possible to download the notebook instead?

I have the last version that pip gave me: 0.8.4.

Thanks!

@mwouts
Copy link
Owner

mwouts commented Oct 30, 2018

Hello @Fubukimaru , thanks for reporting this.

I am afraid that the "Download as > .ipynb" menu entry in Jupyter Notebook stands for "Download current notebook".

I suggest you do the following:

  • pair the py notebook to an ipynb notebook (add a"jupytext_formats":"ipynb,py" metadata)
  • save and close the py notebook
  • and reopen the ipynb notebook. Now the download entry should work as you expect.

Can you confirm that this fixes your issue?

Note that fixing the download menu to always download the ipynb notebook, even when you are editing a py script, would require contributing a fix to Jupyter notebook itself.

@Fubukimaru
Copy link
Author

Hello Marc,

The notebook was already pair as follows:

{
  "jupytext_formats": "ipynb,py",
  "kernelspec": {
    "name": "python3",
    "display_name": "Python 3",
    "language": "python"
  },
....

When opening the .ipynb, the .py gets into running state, even though in the URL it's stated that it's a .ipynb.

Then, when clicking on download, I get the .py file.

Do you have any clue on what could be happening?

Thanks for your support and for jupytext :)!

@mwouts
Copy link
Owner

mwouts commented Oct 31, 2018

Hello Alberto, I played a bit more with this. The good news is that I can reproduce the issue you report!

If I click on "Download as .ipynb" when editing World population.ipynb, I am indeed redirected at http://localhost:8888/nbconvert/ipynb/Documents/GitHub/jupytext/demo/World%20population.pct.py?download=true

I think this is a side effect of merging notebook models. The model returned by Jupytext's contents manager is always that of the source file, cf. https://github.com/mwouts/jupytext/blob/master/jupytext/contentsmanager.py#L270. That structure probably contains the notebook path, and that's why we get .py instead of .ipynb.

I think this is also responsible for a positive side effect, which I want to preserve: Jupyter stops autosaving when the .py file is modified outside of Jupyter, because it looks at the timestamps of that file rather than of the .ipynb.

Let me have a further look at this when time permits - thanks again for reporting!

@Fubukimaru
Copy link
Author

Understood. If you require my assistance, just tell me.

Thanks!

@mwouts mwouts changed the title Download as .ipynb Download as .ipynb returns the associated python file Nov 1, 2018
@mwouts mwouts self-assigned this Nov 1, 2018
@mwouts mwouts added this to the v0.8.5 milestone Nov 1, 2018
mwouts added a commit that referenced this issue Nov 1, 2018
@mwouts
Copy link
Owner

mwouts commented Nov 1, 2018

Alberto, I have pushed a fix for this on branch 0.8.5. Would you like to test it? To install the dev version you will have to:

  • clone the project
  • checkout branch 0.8.5
  • install in dev mode with pip install -e ., once you have cd to the project.

I will also test it myself in the next days, to make sure that it introduces no regression.

@Fubukimaru
Copy link
Author

Hello Marc,

I did the testing. For the use-case it works perfectly 👍.

Now when I open the .py, the .ipynb is marked as running. I can download both .ipynb and .py correctly.

I'll also be testing this version. If I find any drawback, I'll report it.

Thanks!

@mwouts
Copy link
Owner

mwouts commented Nov 2, 2018

Thanks Alberto for testing the new version so quickly!

Two comments:

  • When I download the .py file, what I get is the result of nbconvert applied to the notebook, not Jupytext's representation of the notebook. I think it would be more consistent to offer the option to download the .py file used by Jupytext.
  • I am afraid we are loosing an undocumented, but very useful feature. When (auto)-saving, Jupyter tests the destination file timestamp. Previously it was testing the .py file timestamp, and therefore was aware of modifications on that file and would ask the user to confirm before overwriting. Now it is only watching the .ipynb file, and does overwrite the .py file, even if I started modifying it.

Let me think a bit more on how to improve the user experience on these two additional points... And be careful with the autosave meanwhile! (Turn it off by running %autosave 0 in a cell).

@Fubukimaru
Copy link
Author

About downloading:
I also think the same. Is it feasible?

About saving:
Before opening this issue I didn't know that it didn't autosave, so I was already disabling it with %autosave 0.

What about providing a parameter in the file's metadata to disable autosave? Does it make sense? Does it make sense with the current code?
This way you would maintain the autosave 0 without needing to put the command and execute it every time.

Thanks!

@mwouts mwouts removed this from the v0.8.5 milestone Nov 12, 2018
@mwouts
Copy link
Owner

mwouts commented Nov 12, 2018

Hello Alberto, sorry this is taking a bit longer than expected! I am not yet ready to deliver this, so I moved the code from branch 0.8.5 to notebook_path_is_ipynb_not_py.

The latest update on this:

  • While working on my PyParis talk, I did notice a related issue: when an ipynb is turned into a paired notebook, and refreshed, the kernel does not exist any more (kernel was first associated to ipynb file, but when the notebook is refreshed it is associated to the py file...)
  • Using the ipynb file rather than the py for the name of the Jupyter notebook has the symmetric inconvenient (pairing an ipynb notebook to a py script will change the kernel pointer)
  • We may choose to use the name of the file that was actually opened. But then, it becomes possible to have two kernels on the same notebook!
  • Last piece of the puzzle: Jupyter only checks the timestamp of the file associated to the kernel. Having the py file here is a bit more comfortable, because that is the file we are going to edit outside of Jupyter.

mwouts added a commit that referenced this issue Nov 25, 2018
This allows to
- keep the original Jupyter behavior: the opened file is the one the user did open
- preserve the timestamp test. When saving, Jupyter will test all the timestamps of all formats of the paired notebook.
mwouts added a commit that referenced this issue Nov 25, 2018
This allows to
- keep the original Jupyter behavior: the opened file is the one the user did open
- preserve the timestamp test. When saving, Jupyter will test all the timestamps of all formats of the paired notebook.
@mwouts
Copy link
Owner

mwouts commented Nov 25, 2018

@Fubukimaru , I think I found a solution for this. The file that appears as open will now always be the one that the user did open. And I have added a check on the timestamp: when Jupyter saves a paired notebook, it first checks the timestamps of all alternative representations. Would you mind giving a try to the new release candidate, and confirm that it works as expected? Even in the case when you do not deactivate the autosave? Thanks!

pip install jupytext==0.8.6rc1

@mwouts
Copy link
Owner

mwouts commented Nov 29, 2018

Version 0.8.6 is now available. Thanks @Fubukimaru for reporting this.

@mwouts mwouts closed this as completed Nov 29, 2018
@Fubukimaru
Copy link
Author

Thanks to you. Everything is working perfectly :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants