Closed tomr-stargazer closed 9 years ago
This looks fine to me -- @astrofrog?
Looks good, but I wonder whether we should consider switching to using the astropy progress bar now that it's pretty stable? (to avoid duplication of implementations). Astropy is now a requirement of astrodendro, so is there any reason why not?
@astrofrog Can you point me to an example usage of the astropy progressbar?
The current change is very straightforward since it just replicates the call to the same animated progress bar that dendrogram.py
was already using. I'd be in favor of merging the current (simple) pull request and then looking at the astropy progress bar for future changes, but let me know what you think.
Sounds good - I can make the change later. The Astropy progress bar is described here: http://docs.astropy.org/en/stable/api/astropy.utils.console.ProgressBar.html#astropy.utils.console.ProgressBar
Looks good to me then -- I opened a new issue for replacing this with astropy code
I often compute large dendrograms and catalogs, and while I find the dendrogram animated progress bar invaluable, I realized that I spend way too much time waiting cluelessly for the correspondingly large catalogs to compute. So here's a quick implementation of animated progress bars for catalogs, too.
It looks like this:
The main critique I can foresee of this implementation: maybe instead of overloading the previously-existing
verbose
kwarg inppv_catalog
andpp_catalog
(which currently controls whether the warnings print out or not), I should use a new kwarg (progressbar=True
or something). The advantage of the current usage is that it's parallel toDendogram.compute
's use ofverbose
.