Skip to content

Conversation

lpsinger
Copy link
Contributor

In tqdm.auto, look for IPython in sys.modules rather than explicitly importing it. If we are in an IPython notebook, then IPython will have already been imported. If we are not in an IPython notebook, then importing IPython will take almost half a second.

Here are import times on a 2018 MacBook Pro with an SSD. Before this patch:

$ time python -c 'from tqdm.auto import tqdm'

real     0m0.474s
user     0m0.278s
sys      0m0.142s

After this patch:

$ time python -c 'from tqdm.auto import tqdm'

real    0m0.069s
user    0m0.041s
sys     0m0.020s

Fixes #709.

@lpsinger lpsinger requested a review from casperdcl as a code owner April 29, 2020 14:58
@casperdcl
Copy link
Member

If we are in an IPython notebook, then IPython will have already been imported

Is this guaranteed in all versions?

@casperdcl casperdcl self-assigned this Apr 30, 2020
@casperdcl casperdcl added this to the v5.0.0 milestone Apr 30, 2020
@casperdcl casperdcl added p1-bug-perf ⚡ Speed suboptimal c1-quick 🕐 Complexity low p3-enhancement 🔥 Much new such feature submodule-notebook 📓 Much web such IDE to-review 🔍 Awaiting final confirmation labels Apr 30, 2020
@lrq3000
Copy link
Member

lrq3000 commented Apr 30, 2020

Awesome idea! Thank you for the PR!

As @casperdcl says, need to double check whether IPython may have been renamed to jupyter in the latest releases. I have just tested with jupyter v7.13.0 (the latest stable version) and it works the same :-) This convention should not change in the foreseeable future since they are now working on v8. They used the same module name (IPython instead of jupyter) since years now, probably for retrocompatibility of old notebooks, I doubt they will change on a whim now.

@lpsinger
Copy link
Contributor Author

lpsinger commented May 1, 2020

As @casperdcl says, need to double check whether IPython may have been renamed to jupyter in the latest releases.

If that renaming occurred, then it would break the existing code too.

@lpsinger
Copy link
Contributor Author

lpsinger commented May 3, 2020

I asked on https://gitter.im/jupyter/jupyter and got this answer from @MSeal:

@lpsinger I dug into this topic a while back to figure out what the constraints / guarantees are as this comes up a lot in issues on github. This is the closest to detecting that you're in an ipython kernel accurately as you can do today: https://github.com/nteract/scrapbook/blob/master/scrapbook/utils.py#L46-L57. It's likely to continue working in IPython 8, but there's not a strong guarantee there.

@casperdcl
Copy link
Member

hmm we go a little further and do 'IPKernelApp' not in get_ipython().config rather than just get_ipython().kernel is not None. Not sure which is better. Also note I think there isn't meant to be a way to distinguish between an ipy terminal and notebook, which makes things harder...

@MSeal
Copy link

MSeal commented May 5, 2020

'IPKernelApp' not in get_ipython().config rather than just get_ipython().kernel is not None

Probably equivalent in forward compatibility.

Also note I think there isn't meant to be a way to distinguish between an ipy terminal and notebook

As I understand it, there's not meant to be a way to distinguish if you're in a ipykernel that's being controlled by a notebook or not. I don't believe there was an explicit emphasis on hiding if you're in ipython or ipykernel, but there certainly isn't any helper.

@lpsinger
Copy link
Contributor Author

lpsinger commented Jun 3, 2020

Any updates on this?

@casperdcl
Copy link
Member

will likely be in tqdm>=4.47.0; thanks for following up

@lpsinger
Copy link
Contributor Author

Is it possible to include this in v4.47.0 now that issue #993 has been created? I am having to use my branch for performance reasons in another project (http://git.ligo.org/emfollow/gwcelery).

@casperdcl
Copy link
Member

Yes, it'll be in #993...

@casperdcl casperdcl mentioned this pull request Jun 26, 2020
1 task
casperdcl and others added 2 commits June 26, 2020 16:51
Would be required to extend queue len > 2
In tqdm.auto, look for `IPython` in `sys.modules` rather than
explicitly importing it. If we are in an IPython notebook, then
IPython will have already been imported. If we are not in an
IPython notebook, then importing IPython will take almost half a
second.

Here are import times on a 2018 MacBook Pro with an SSD.
Before this patch:

    $ time python -c 'from tqdm.auto import tqdm'

    real     0m0.474s
    user     0m0.278s
    sys      0m0.142s

After this patch:

    $ time python -c 'from tqdm.auto import tqdm'

    real    0m0.069s
    user    0m0.041s
    sys     0m0.020s

Fixes tqdm#709.
@casperdcl casperdcl changed the base branch from master to devel June 28, 2020 19:18
@casperdcl casperdcl force-pushed the tqdm-auto-sys-modules branch from 92ec6e6 to 4e8f74c Compare June 28, 2020 19:20
@codecov-commenter
Copy link

Codecov Report

Merging #955 into devel will increase coverage by 1.62%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            devel     #955      +/-   ##
==========================================
+ Coverage   85.41%   87.04%   +1.62%     
==========================================
  Files          22       21       -1     
  Lines        1303     1250      -53     
  Branches      219      213       -6     
==========================================
- Hits         1113     1088      -25     
+ Misses        169      141      -28     
  Partials       21       21              

@casperdcl casperdcl merged commit 2e49514 into tqdm:devel Jun 28, 2020
@lpsinger
Copy link
Contributor Author

Thank you!

@lpsinger lpsinger deleted the tqdm-auto-sys-modules branch June 30, 2020 20:06
lpsinger added a commit to lpsinger/gwcelery that referenced this pull request Jun 30, 2020
tqdm/tqdm#955 has been merged and is now
in a release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c1-quick 🕐 Complexity low p1-bug-perf ⚡ Speed suboptimal p3-enhancement 🔥 Much new such feature submodule-notebook 📓 Much web such IDE to-review 🔍 Awaiting final confirmation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

importing tqdm.auto is very slow due to ipython completion initialization
5 participants