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

Clear_output: Animation & widget related changes. #4229

Merged
merged 8 commits into from Sep 23, 2013

Conversation

jdfreder
Copy link
Member

  • No more delay when calling clear_output
  • Removed ability to clear stderr and stdout individually.
  • Output div height is unmodified
  • New wait flag prevents clear_output from being executed until new output is available

Animation flickering also mentioned in #1563

Some of the behaviors that were unwanted can be seen in this YouTube video.

@minrk
Copy link
Member

minrk commented Sep 19, 2013

review from dev meeting: remove the height-fixing when wait=False

@jdfreder
Copy link
Member Author

remove the height-fixing when wait=False

Thanks, changes made ^

@ellisonbg
Copy link
Member

The code looks great, but can you update our example notebook that shows how to use this stuff:

https://github.com/ipython/ipython/blob/master/examples/notebooks/Animations%20Using%20clear_output.ipynb

@jdfreder
Copy link
Member Author

@ellisonbg thanks, changes made ^

@ellisonbg
Copy link
Member

A few other places where we should probably use wait=True:

./IPython/parallel/client/asyncresult.py:
   26 : from IPython.core.display import clear_output, display, display_pretty
  394 :             clear_output()
./examples/parallel/InteractiveMPI-publish-data.ipynb:
  237 :       "from IPython.display import display, clear_output\n",
  248 :       "        By default it calls clear_output so that new plots replace old ones.  Set\n",
  274 :       "            clear_output()\n",
  336 :       "        clear_output()\n",
./examples/parallel/InteractiveMPI.ipynb:
  252 :       "from IPython.display import clear_output\n",
  261 :       "        By default it calls clear_output so that new plots replace old ones.  Set\n",
  284 :       "        clear_output()\n",
  380 :       "        clear_output()\n",

You will need MPI to run those last two notebooks - don't worry about that - just update the code.

@ellisonbg
Copy link
Member

This PR should also add an entry in the what's new in /docs/source/whatsnew.

@jdfreder
Copy link
Member Author

@ellisonbg thanks, changes made again ^


* There is no longer a 500ms delay when calling ``clear_output``.
* The ability to clear stderr and stdout individually was removed.
* A new wait flag that prevents ``clear_output`` from being executed until new
Copy link
Member

Choose a reason for hiding this comment

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

put wait in double backticks here.

@jdfreder
Copy link
Member Author

Thanks @ellisonbg , changes made

@ellisonbg
Copy link
Member

OK everything is done, merging.

ellisonbg added a commit that referenced this pull request Sep 23, 2013
Clear_output: Animation & widget related changes.
@ellisonbg ellisonbg merged commit 696bf69 into ipython:master Sep 23, 2013
@@ -300,6 +305,7 @@ var IPython = (function (IPython) {
this.append_stream(json);
}
this.outputs.push(json);
this.element.height('auto');
Copy link
Member

Choose a reason for hiding this comment

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

is this auto height the right behavior, or was that a remnant from the previous implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

It still measures/sets the height if wait=True, this line is supposed to reset that. So yes & yes.

If parsing javascript threadlocks the browser's render function, this is pointless since it wont redraw in between clear_output and the append operation. Do you have insight to whether or not this is the case?

@Carreau
Copy link
Member

Carreau commented Sep 27, 2013

Ahhhhhhhh !!!!!

this.element.height('auto');

I cannot have output that scroll now.... the scoll area cannot be smaller than the output anymooooooooooore

run

for i in range(1000):
    print i

and now... suffer !

@jdfreder
Copy link
Member Author

I cannot have output that scroll now....

Shoot, that's a problem... If I put a conditional around the auto-height that should be enough to fix the problem. I doubt you want scrolling output for rapid animations, right?

Carreau added a commit that referenced this pull request Oct 4, 2013
Fix scrolling output (not working post clear_output changes)

Regression introduced in #4229
jdfreder added a commit to jdfreder/ipython that referenced this pull request Dec 19, 2013
Regression introduced in ipython#4229
jdfreder added a commit to jdfreder/ipython that referenced this pull request Dec 20, 2013
Regression introduced in ipython#4229
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Clear_output: Animation & widget related changes.
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Fix scrolling output (not working post clear_output changes)

Regression introduced in ipython#4229
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
Regression introduced in ipython#4229
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.

None yet

4 participants