search

kynan / nbstripout

strip output from Jupyter and IPython notebooks
Other
1.21k stars 95 forks source link

diff.ipynb.textconv fails with TortoiseGit on Windows #80

Closed ibressler closed 6 years ago

ibressler commented 6 years ago

When showing the diff (or patch) in TortoiseGit GUI on Windows (Anaconda3 Distribution) diff.ipynb.textconv fails with an error message:

Traceback (most recent call last):
  File "C:/Users/ibressle/AppData/Local/Continuum/anaconda3/lib/site-packages/nbstripout.py", line 109, in <module>
    input_stream = io.TextIOWrapper(sys.stdin.buffer, encoding='utf-8')
AttributeError: 'NoneType' object has no attribute 'buffer'
fatal: unable to read files to diff

To reproduce, using the Explorer context menu, right-click on a file with changes, select 'Git Commit', in the dialog, double-click the file to show the patch. In the separate patch window the error message from above appears and the presented diff contains the ipynb code with outputs respectively.

Besides, the filter for commits seems to work fine because the stripped ipynb is committed finally. Furthermore, it works fine on the console ('Git Bash' from Explorer context menu provided by TortoiseGit).

kynan commented 6 years ago

Which Python version are you using? Since GUI apps don't have a stdin stream by default sys.stdin can be None in some Python versions, though the behaviour has changed in the CPython trunk.

As long as you don't try to then read from stdin it should be fine not to create the input_stream wrapper. Are you interested to work on a pull request @ibressler ?

kynan commented 6 years ago

Unfortunately I don't have a Windows machine where I could test this.

ibressler commented 6 years ago

I had the issue with Python 3.6.5 (Anaconda3 5.2.0 64bit) on Win10. To fix this, I added pull request #81 which connects the standard streams to os.devnull if they are None initially. Alternatively, it could also be fixed by careful handling of input_stream along the way, as you suggest (?). Setting input_stream=None if stdin is None and avoiding any code using input_stream by testing for None. Is this what you refer to?

kynan commented 6 years ago

I've added a comment on #81: my suggestion would have been to defer creating input_stream until we know it's needed and then check if stdin is available before creating the wrapper. I'm equally happy with your solution though!