Skip to content

Fix jupyroot on Windows #9107

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.

Sign up for GitHub

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

Closed
bellenot wants to merge 2 commits into from
Closed

Fix jupyroot on Windows #9107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8cf3a0c
Fix jupyroot on Windows
bellenot Oct 12, 2021
02a9a3a
Fix "access denied" error with png images on Windows
bellenot Oct 13, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions bindings/jupyroot/python/JupyROOT/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
# For the list of contributors see $ROOTSYS/README/CREDITS. #
################################################################################

from JupyROOT.helpers import cppcompleter, utils
import sys
from JupyROOT.helpers import utils
if not 'win32' in sys.platform:
from JupyROOT.helpers import cppcompleter

# Check if we are in the IPython shell
try:
Expand All @@ -21,6 +24,7 @@
_is_ipython = hasattr(builtins, '__IPYTHON__')

if _is_ipython:
from IPython import get_ipython
cppcompleter.load_ipython_extension(get_ipython())
if not 'win32' in sys.platform:
Copy link
Copy Markdown
Contributor

@etejedor etejedor Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only effect of this is that we won't have tab completion for now, correct?

Copy link
Copy Markdown
Member Author

@bellenot bellenot Oct 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct

from IPython import get_ipython
cppcompleter.load_ipython_extension(get_ipython())
utils.iPythonize()
10 changes: 7 additions & 3 deletions bindings/jupyroot/python/JupyROOT/helpers/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
import sys
import select
import tempfile
import pty
if not 'win32' in sys.platform:
Copy link
Copy Markdown
Contributor

@etejedor etejedor Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked an pty is actually not used anywhere in this file anymore, so just a stale import. You can perhaps remove it altogether.

Copy link
Copy Markdown
Member Author

@bellenot bellenot Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine with me

import pty
import itertools
import re
import fnmatch
Expand Down Expand Up @@ -561,10 +562,12 @@ def _jsDisplay(self):
return 0

def _getPngImage(self):
ofile = tempfile.NamedTemporaryFile(suffix=".png")
ofile = tempfile.NamedTemporaryFile(suffix=".png", delete=False)
Copy link
Copy Markdown
Contributor

@etejedor etejedor Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to prevent deletion via the destruction of the NamedTemporaryFile object and delete the underlying file explicitly later?

Copy link
Copy Markdown
Member Author

@bellenot bellenot Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To prevent the following error:

Error in callback <bound method CaptureDrawnPrimitives._post_execute of <JupyROOT.helpers.utils.CaptureDrawnPrimitives object at 0x0C321C58>> (for post_execute):
---------------------------------------------------------------------------
PermissionError                           Traceback (most recent call last)
~\build\release\bin\JupyROOT\helpers\utils.py in _post_execute(self)
    461 
    462     def _post_execute(self):
--> 463         NotebookDraw()
    464 
    465     def register(self):

~\build\release\bin\JupyROOT\helpers\utils.py in NotebookDraw()
    450 def NotebookDraw():
    451     DrawGeometry()
--> 452     DrawCanvases()
    453     DrawRCanvases()
    454 

~\build\release\bin\JupyROOT\helpers\utils.py in DrawCanvases()
    441     drawers = GetCanvasDrawers()
    442     for drawer in drawers:
--> 443         drawer.Draw()
    444 
    445 def DrawRCanvases():

~\build\release\bin\JupyROOT\helpers\utils.py in Draw(self)
    598 
    599     def Draw(self):
--> 600         self._display()
    601         return 0
    602 

~\build\release\bin\JupyROOT\helpers\utils.py in _display(self)
    583             self._jsDisplay()
    584          else:
--> 585             self._pngDisplay()
    586 
    587     def GetDrawableObjects(self):

~\build\release\bin\JupyROOT\helpers\utils.py in _pngDisplay(self)
    572 
    573     def _pngDisplay(self):
--> 574         img = self._getPngImage()
    575         IPython.display.display(img)
    576 

~\build\release\bin\JupyROOT\helpers\utils.py in _getPngImage(self)
    566         with _setIgnoreLevel(ROOT.kError):
    567             self.drawableObject.SaveAs(ofile.name)
--> 568         img = IPython.display.Image(filename=ofile.name, format='png', embed=True)
    569         #ofile.close()
    570         #os.unlink(ofile.name)

c:\python38-32\lib\site-packages\IPython\core\display.py in __init__(self, data, url, filename, format, embed, width, height, retina, unconfined, metadata)
   1229         self.retina = retina
   1230         self.unconfined = unconfined
-> 1231         super(Image, self).__init__(data=data, url=url, filename=filename, 
   1232                 metadata=metadata)
   1233 

c:\python38-32\lib\site-packages\IPython\core\display.py in __init__(self, data, url, filename, metadata)
    635             self.metadata = {}
    636 
--> 637         self.reload()
    638         self._check_data()
    639 

c:\python38-32\lib\site-packages\IPython\core\display.py in reload(self)
   1261         """Reload the raw data from file or URL."""
   1262         if self.embed:
-> 1263             super(Image,self).reload()
   1264             if self.retina:
   1265                 self._retina_shape()

c:\python38-32\lib\site-packages\IPython\core\display.py in reload(self)
    660         """Reload the raw data from file or URL."""
    661         if self.filename is not None:
--> 662             with open(self.filename, self._read_flags) as f:
    663                 self.data = f.read()
    664         elif self.url is not None:

PermissionError: [Errno 13] Permission denied: 'C:\\Users\\sftnight\\AppData\\Local\\Temp\\tmpk49jx00w.png'

See the documentation of NamedTemporaryFile in tempfile:

Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms (it can be so used on Unix; it cannot on Windows NT or later).

Copy link
Copy Markdown
Contributor

@etejedor etejedor Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see thanks!

with _setIgnoreLevel(ROOT.kError):
self.drawableObject.SaveAs(ofile.name)
img = IPython.display.Image(filename=ofile.name, format='png', embed=True)
ofile.close()
os.unlink(ofile.name)
return img

def _pngDisplay(self):
Expand Down Expand Up @@ -610,7 +613,8 @@ def loadMagicsAndCapturers():
extMgr = ExtensionManager(ip)
for extName in extNames:
extMgr.load_extension(extName)
captures.append(StreamCapture())
if not 'win32' in sys.platform:
Copy link
Copy Markdown
Contributor

@etejedor etejedor Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So atm no text output is seen in the notebook if it comes from C++ code on Windows, is this right?

Copy link
Copy Markdown
Member Author

@bellenot bellenot Oct 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right

captures.append(StreamCapture())
captures.append(CaptureDrawnPrimitives())

for capture in captures: capture.register()
Expand Down