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

ProgressMeter doesn't work in jupyter lab #2078

Open
kain88-de opened this Issue Sep 18, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@kain88-de
Member

kain88-de commented Sep 18, 2018

Expected behavior

A progress meter works the same in a jupyter lab environment and an ipython terminal.

Actual behavior
screenshot from 2018-09-18 15-16-55

Currently version of MDAnalysis

  • Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)") 0.18.0
  • Which version of Python (python -V)? 3.6
  • Which operating system? Linux
@orbeckst

This comment has been minimized.

Member

orbeckst commented Sep 18, 2018

Is JupyterLab different from a Jupyter notebook? It used to work in the latter, didn't it?

@kain88-de

This comment has been minimized.

Member

kain88-de commented Sep 18, 2018

@IAlibay

This comment has been minimized.

Contributor

IAlibay commented Sep 19, 2018

So had a look at this as a nice excuse to wrap my head around the progress meter. It looks like Jupyter Lab doesn't like progress meter formatting assigned by the RMSD class. In particular, it breaks on the use of \r in "{step:5d}/{numsteps} [{percentage:5.1f}%]\r" (line 607).

Removing it seems to fix this issue:

image

This would explain why the progress meter for u.transfer_to_memory() works properly.

Within analysis, it looks this will also affect density_from_Universe (checked), helanal_trajectory, PCA, HydorgenBondAnalysis, HydrogenBondAutoCorrel, and WaterBridgeAnalysis.

I have tested removing the extra carriage return on RMSD and density_from_Universe, and (on python3) it looks like it works fine across jupyterlab, ipython and standard python calls.

@kain88-de

This comment has been minimized.

Member

kain88-de commented Sep 19, 2018

@IAlibay

This comment has been minimized.

Contributor

IAlibay commented Sep 19, 2018

@kain88-de Will do, I'm still trying to work out if anything outside analysis is affected but I'll create the PR once I'm done (and tested on p2.7).

@orbeckst

This comment has been minimized.

Member

orbeckst commented Nov 7, 2018

@jbarnoud 's PR #960 is still hanging somewhere in limbo.

I also only recently discovered tqdm (which probably everyone else already knows...) and we could also think about adapting it instead of our home-grown approach. It works in notebooks and is a light-weight dependency.

@IAlibay

This comment has been minimized.

Contributor

IAlibay commented Nov 7, 2018

Just as an update on the original fix for this, I did have most of this fixed (I think I'm missing a test for some helanal changes related to #2084), just ended up getting distracted with other work. Sorry about that. I'll try to open the PR for it over the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment