Skip to content

Fix jupyroot on Windows#9107

Closed
bellenot wants to merge 2 commits intoroot-project:masterfrom
bellenot:fix-jupyroot-windows
Closed

Fix jupyroot on Windows#9107
bellenot wants to merge 2 commits intoroot-project:masterfrom
bellenot:fix-jupyroot-windows

Conversation

@bellenot
Copy link
Member

No description provided.

@bellenot bellenot requested a review from etejedor October 12, 2021 14:50
@bellenot bellenot self-assigned this Oct 12, 2021
@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, ROOT-ubuntu2004/soversion, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2021

This pull request fixes 1 alert when merging 02a9a3a into 4449ef4 - view on LGTM.com

fixed alerts:

  • 1 for Unused import


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

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
Member Author

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
Contributor

Choose a reason for hiding this comment

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

Ok I see thanks!

return True

_enableJSVis = False
if 'win32' in sys.platform:
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is done by this commit and undone by the next one. Is this intended? Then I propose the change is just removed from both commits.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is intended.

import select
import tempfile
import pty
if not 'win32' in sys.platform:
Copy link
Contributor

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
Member Author

Choose a reason for hiding this comment

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

Fine with me

for extName in extNames:
extMgr.load_extension(extName)
captures.append(StreamCapture())
if not 'win32' in sys.platform:
Copy link
Contributor

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
Member Author

Choose a reason for hiding this comment

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

right

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

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
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.

Correct

@phsft-bot
Copy link

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

bellenot added a commit to bellenot/root that referenced this pull request Oct 13, 2021
 - Remove unused `import pty`
 - Disable stream capture (currently not working on Windows). The output coming from C++ (ROOT) will only print in the command prompt and not in the notebook (to be investigated)
 - Disable tab completion (currently not working on Windows).
 - Add the `delete=False` argument in `NamedTemporaryFile()` and manually delete the created file 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'
    ```
    For the details, see the documentation of `NamedTemporaryFile` in [tempfile](https://docs.python.org/3/library/tempfile.html):
    ```
    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).
    ```
@bellenot
Copy link
Member Author

Superseded by #9109

@bellenot bellenot closed this Oct 13, 2021
bellenot added a commit that referenced this pull request Oct 13, 2021
 - Remove unused `import pty`
 - Disable stream capture (currently not working on Windows). The output coming from C++ (ROOT) will only print in the command prompt and not in the notebook (to be investigated)
 - Disable tab completion (currently not working on Windows).
 - Add the `delete=False` argument in `NamedTemporaryFile()` and manually delete the created file 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'
    ```
    For the details, see the documentation of `NamedTemporaryFile` in [tempfile](https://docs.python.org/3/library/tempfile.html):
    ```
    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).
    ```
@bellenot bellenot deleted the fix-jupyroot-windows branch February 7, 2022 14:26
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

Successfully merging this pull request may close these issues.

3 participants