Consider replacing astropy's progress bar machinery with tqdm

@adrn

tqdm is a package that implements easy/pretty progress bars in python. While we have something like this in astropy.utils.console, it might be wise to switch to tqdm in the interest of better code sharing/reuse.

One concern is how hard it will be to adapt our machinery to using this. If the interface is too different it's probably not worth it.

Another concern that needs checking is whether or not the notebook progress bar in tqdm is up-to-snuff (because the astropy one works quite well).

Originally suggested by @adrn

I would like to offer a different viewpoint -- "If it ain't broken, don't fix it." Is it worth adding yet another dependency just for a prettier progress bar?

The reason is not that it's prettier, but rather that it doesn't really make sense for astropy to maintain a progress bar utility. For the sake of the open-source community and the modularity of astropy, I think it would make sense to switch. Of course, this should be weighed with the practicality of switching...

Just to be clear, would we bundle it or have it as a dependency?

@astrofrog

@astrofrog I would say dependency. If tqdm is not installed, I think the user could live without a progress bar? The Astropy ProgressBar class is only used in astropy/utils/data.py and astropy/utils/data.py

To say it is "only used" is misleading. Whenever remote data is downloaded, you will see that progress bar. I still think it's a bit excessive to require an extra dependency for a low-level util like this, but that's just me.

I think it is a good idea to swap, since really we should not maintain that kind of code, but on the other hand I do think the default should be to have a progress bar without having to depend on the user installing it. Sincetqdm is not exactly large, (indeed, it seems fairly similar to our current console.py), could we not just bundle it?

I wanted to use the Astropy progress update utils today for the first time.
The use case is to report to users progress of HiPS tile fetching from the internet and tile drawing to make sky images: hipspy/hips#80

So I looked at the examples here:

Some observations / feedback as a naive first-time user:

None of the examples given is complete that I can just copy & paste them and try them out.
The spinner example calls next on a generator, which doesn't work on Python 3 where it's __next__, this needs to be fixed.
Why is it bar.update() and s.next()? This difference is unnecessary and hard to remember, no?
By default in the Jupyter notebook there's no output at all from the progress bar. Wouldn't it be possible / better default behaviour to show text output like on the terminal? I'm not sure how to use the progress bar from my hips package so that it shows nice progress for users both from the console and Jupyter (and some progress even in cases where they don't have the Jupyter widgets enabled like I have at the moment).

This is the full example that I used in the end to try it, we could put that in the docstring:

In [3]: %paste
from astropy.utils.console import ProgressBar
import time

items = range(10)
for item in ProgressBar(items):
    # Do some computation with `item` here
    time.sleep(0.2)

## -- End pasted text --
|================================================================================================================|  10 / 10  (100.00%)         2s

The discussion here has stalled over a year ago. I can see how this is a tough decision. Is there a way to resolve the discussion here and make a decision what to do?

Some of the comments above lean towards switching out the implementation and (if I understand correctly), change the implementation of the existing classes to be wrappers around tqdm and provide backward compatibility for users of the astropy.utils. Is it possible? Is it desirable given the point I made about the spinner and progress bar API above or would complete deprecation of this part of astropy.utils be better (I mean via the docs, i.e. just say that external packages like hips should consider using something like tqdm directly)?

If I get some feedback here, I could make a PR fixing / improving the docstrings of Spinner and ProgressBar a bit, but I don't have time to e.g. try switching to tqdm (and I'm not familiar with the implementations at all).

I think in general this is a good idea. I mean the "simplest" solution is to change all internal uses of progress bar to tqdm and then just deprecate the existing implementation?

Just wanted to bring this up again after 8 years :)
tqdm is pretty much used everywhere now, and I am not sure how actively is the ProgressBar inside astropy is maintained. I tried switching to tqdm in astroquery and it shouldn't be that big of difference for downstream users (IMO).

With astropy's progressbar:

With tqdm:

Let me know if there is interest in pursuing it :)

The more controversial discussion is adding it as a required dependency. We can't even agree on Scipy...

Make scipy a required dependency #12448

And even older but broader discussions:

Of required and optional dependencies #10342

we should make it a dependency. we should it duke it out to see which side prevails :3

@MridulS

The main opposition to scipy surrounds how "big" its install is and how complicated it can be to build from source. tqdm appears to be ~78kB and entirely Python code with no serious dependencies itself. So I think this is an entirely reasonable thing to do. Especially if @MridulS is correct in it being a fairly simple change.

Hmm... Where exactly?

astropy/astropy/utils/data.py

Lines 1749 to 1759 in 884bcac

    
           combined_paths = ProgressBar.map( 
        
               _do_download_files_in_parallel, 
        
               [ 
        
                   dict( 
        
                       remote_url=u, 
        
                       cache=cache, 
        
                       show_progress=False, 
        
                       timeout=timeout, 
        
                       sources=sources.get(u, None), 
        
                       pkgname=pkgname, 
        
                       temp_cache=astropy.config.paths.set_temp_cache._temp_path,

astropy/astropy/io/votable/validator/main.py

Line 136 in 884bcac

ProgressBar.map(download, args, multiprocess=multiprocess)

It sounds like this may be more work than is worth it for something that isn't broken...

Right, the validator. Yeah, I remember now. It was designed to download multiple sources at once for validation. If PyVO eventually takes it over, maybe it is not such a big deal for astropy.

https://astroconda.org/aux/vo_databases/daily_html/

But the one in download_files_in_parallel would be tough to replace, since astropy.utils.data stuff is widely used downstream.

Does tqdm not support multiprocessing at all? That is a shame.

Should we just close this as won't fix then?

Does tqdm not support multiprocessing at all? That is a shame.

It does but it is just a progress bar, you would still need to write the actual code to do the multiprocessing bits. Astropy's porgressbar isn't really named properly, it's doing a lot more than just a progress bar 😅

Should we just close this as won't fix then?

Yeah I think we can just close this issue, let's keep it #4738 (comment) till it starts annoying us.

It does but it is just a progress bar, you would still need to write the actual code to do the multiprocessing bits. Astropy's porgressbar isn't really named properly, it's doing a lot more than just a progress bar 😅

I've used tqdm with multiprocessing - and for simple use cases it's been as easy as just setting up a multiprocessing.Pool so it might not be too much work.

@astrofrog

@astrofrog , what are you suggesting here in terms of actionable item?

Since both cases are dealing with parallel download, it would probably be better to use a thread pool (e,g, concurrent.futures) instead of a process one. (Or some package dedicated to parallel download, e.g. https://github.com/Cadair/parfive but maybe overkill for just those 2 cases)

I would be more comfortable if parfive isn't in Stuart's personal account and has recent releases...

If we are serious about parfive, we are talking about requiring also tqdm and aiohttp.

https://github.com/Cadair/parfive/blob/3b049c55f5cd35a607aafdfecfbccabf7bc71e29/setup.cfg#L19-L21

I'm down to code whatever needs to be done.

@pllim

@pllim - not really serious, for 2 simple cases using directly a thread pool would be simpler.

@mhvk

I agree we should deprecate our ProgressBar and also that it's not as trivial a job as we'd like. I would suggest:

rewriting the internals of ProgressBar to use tqdm. Users see no change from this except if they don't have tqdm installed, in which case we should raise a warning.
Deprecating the ProgressBar, splitting out any functionality we need to keep , e.g. for download_files_in_parallel, as private functions.

It's true that the code still works, but it's also true that this isn't the type of thing we want to maintain. Rather than waiting for it to eventually break, as everything does eventually, it would be much better if we actively switched to the de facto standard in Python.

I also agree with @mhvk that it would be great if astropy[recommended] could be the default. I hope Python implements that soon.

@tactipus. PRs welcome!!

I still don't see much point in introducing tqdm if we cannot get rid of anything in the end. Progress bar tech isn't going to change much and it is currently working.

At the very least we could always consider just recommending to users in the docs to consider using tqdm instead.

I would be more comfortable if parfive isn't in Stuart's personal account and has recent releases...

ouch 😛

It got a bit of love recently, it's actually reasonably active for such a project which does what it intends to.

If we are serious about parfive, we are talking about requiring also tqdm and aiohttp.

I am considering ditching aiohttp in favour of httpx as it's a little lighter weight (aiohttp is also a full http server library). If that's enough to make a difference then I would bump it up the list .

Navigation Menu

Repository navigation

Consider replacing astropy's progress bar machinery with tqdm #4738

24 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	combined_paths = ProgressBar.map(
	_do_download_files_in_parallel,
	[
	dict(
	remote_url=u,
	cache=cache,
	show_progress=False,
	timeout=timeout,
	sources=sources.get(u, None),
	pkgname=pkgname,
	temp_cache=astropy.config.paths.set_temp_cache._temp_path,

Consider replacing astropy's progress bar machinery with tqdm #4738

Description

Activity

pllim commented on Mar 28, 2016

adrn commented on Mar 28, 2016

astrofrog commented on Apr 19, 2016

adrn commented on Jul 2, 2016

pllim commented on Jul 5, 2016

mhvk commented on Jul 5, 2016

cdeil commented on Jul 26, 2017

Cadair commented on Jul 26, 2017

MridulS commented on Feb 20, 2024

pllim commented on Feb 20, 2024

tactipus commented on Feb 20, 2024

WilliamJamieson commented on Feb 20, 2024

24 remaining items

MridulS commented on Feb 23, 2024

mhvk commented on Feb 23, 2024

pllim commented on Feb 23, 2024

MridulS commented on Feb 24, 2024

astrofrog commented on Feb 24, 2024

pllim commented on Feb 26, 2024

saimn commented on Mar 6, 2024

pllim commented on Mar 6, 2024

tactipus commented on Mar 6, 2024

saimn commented on Mar 6, 2024

nstarman commented on Apr 15, 2024

pllim commented on Apr 15, 2024

astrofrog commented on Apr 15, 2024

Cadair commented on Apr 16, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions