-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Support interactive graphics in python scripts #17587
base: master
Are you sure you want to change the base?
Conversation
Test Results 18 files 18 suites 4d 3h 52m 28s ⏱️ For more details on these failures, see this check. Results for commit 0c77f7c. ♻️ This comment has been updated with latest results. |
@linev and what about the browser? |
If approach works with canvas, one can provide similar solution for |
Can you check PR? |
@linev thank you for this PR! I believe it is improving a situation for a specific use case of our Python interface, that's good! Let me summarise my findings from a few quick tests I ran locally.
import ROOT
c = ROOT.TCanvas()
h = ROOT.TH1D("h","h",100,-10,10)
h.Draw()
c.Draw()
c.Update()
ROOT.gSystem.ProcessEvents()
for _ in range(1000):
h.FillRandom("gaus",100)
h.Draw()
c.Update()
ROOT.gSystem.ProcessEvents() which will be broken with the current changes. As discussed offline, this can be accommodated by adding a Python-only Let me note that this example I came up with is just one, there could be many more. I wish there was a better way we could expedite this kind of testing. Notably, I have tried these changes at a Python prompt, at a IPython prompt and in a Jupyter notebook. Everything seems to work as intended there, but having to do this manually is unfortunate. |
I add extra |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, I left some comments. I still believe the Draw
method sees more usage than Update
.
First invoke normal canvas Update. | ||
If block == True specified, run ROOT event loop until <space> key is pressed | ||
Event loop does not processed in batch mode or in ipython |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First invoke normal canvas Update. | |
If block == True specified, run ROOT event loop until <space> key is pressed | |
Event loop does not processed in batch mode or in ipython | |
Updates the canvas. Also blocks script execution and runs the ROOT graphics event loop until the <space> keyword is pressed, but only if the following conditions are met: | |
* The `block` optional argument is set to `True`. | |
* ROOT graphics are enabled, i.e. `ROOT.gROOT.IsBatch() == False`. | |
* The script is being run in non-interactive mode, i.e. `python myscript.py`. |
# import atexit | ||
|
||
# def cleanup(): | ||
# # If spawned, stop thread which processes ROOT events | ||
# facade = sys.modules[__name__] | ||
# if "app" in facade.__dict__ and hasattr(facade.__dict__["app"], "process_root_events"): | ||
# facade.__dict__["app"].keep_polling = False | ||
# facade.__dict__["app"].process_root_events.join() | ||
|
||
def cleanup(): | ||
# If spawned, stop thread which processes ROOT events | ||
facade = sys.modules[__name__] | ||
if "app" in facade.__dict__ and hasattr(facade.__dict__["app"], "process_root_events"): | ||
facade.__dict__["app"].keep_polling = False | ||
facade.__dict__["app"].process_root_events.join() | ||
|
||
atexit.register(cleanup) | ||
# atexit.register(cleanup) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove all the commented code
def _process_root_events(self): | ||
while self.keep_polling: | ||
gSystem.ProcessEvents() | ||
time.sleep(0.01) | ||
import threading | ||
self.keep_polling = True # Used to shut down the thread safely at teardown time | ||
update_thread = threading.Thread(None, _process_root_events, None, (self,)) | ||
self.process_root_events = update_thread # The thread is joined at teardown time | ||
update_thread.daemon = True | ||
update_thread.start() | ||
# def _process_root_events(self): | ||
# while self.keep_polling: | ||
# gSystem.ProcessEvents() | ||
# time.sleep(0.01) | ||
# import threading | ||
# self.keep_polling = True # Used to shut down the thread safely at teardown time | ||
# update_thread = threading.Thread(None, _process_root_events, None, (self,)) | ||
# self.process_root_events = update_thread # The thread is joined at teardown time | ||
# update_thread.daemon = True | ||
# update_thread.start() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here please remove commented code
from . import pythonization | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the usual copyright header here and I would also add some documentation that will end up in the Python Interface section of the docs https://root.cern.ch/doc/master/group__Pythonizations.html. Take for example https://github.com/root-project/root/blob/master/bindings/pyroot/pythonizations/python/ROOT/_pythonization/_tfile.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I apply your changes and rebase commits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are still missing the documentation as I suggested, see the \pythondoc
block in _tfile.py
. I think it should describe the fact that the Update
method has been pythonized and a simple example usage.
If extra `block` flag specified, python script execution is blocked until <space> key is pressed. At the same time ROOT events loop continue running. Solves problem of ROOT graphics usage in non-interactive python
While now events processing has to be done in the canvas.Update method, additional processing is not necessary. Such approach was not thread safe and did not work on Windows at all
It is interrupt flag, which can be triggered from canvas context menu. Has similar effect as prssing space button
from . import pythonization | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are still missing the documentation as I suggested, see the \pythondoc
block in _tfile.py
. I think it should describe the fact that the Update
method has been pythonized and a simple example usage.
Yes, I add small docu and example macro |
Currently it is problematic to use simple canvas (web or not-web) in non-interactive python.
Either macro exit very fast or created canvas is not possible to use interactively.
I propose to provide pythonization of
TCanvas.Update()
method which can block macro execution and wait untiluser press
<space>
key. While python waits for key press ROOT event loop continues to run and interactive canvas is fully functional.There is additional
block
parameter ofcanvas.Update()
now which is False by default.Without extra arguments it behaves as normal ROOT. Only when
block = True
, method blocks until key pressed.Simple example of usage:
In batch mode or in python notebooks blocking will not be performed.
Tested on:
The only difference between Windows and other platforms - in interactive python mode one
cannot use special hook to perform periodic actions while python prompt is waiting for next user command.
Therefore on Windows to work with canvas one always have to invoke
c.Update(True)
This PR resolves #14943 and #13744