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

Progressbar for ESO get_header #3000

Open
m-samland opened this issue Apr 29, 2024 · 5 comments
Open

Progressbar for ESO get_header #3000

m-samland opened this issue Apr 29, 2024 · 5 comments

Comments

@m-samland
Copy link

I am working on a package that requires downloading a lot of headers for files from the ESO archive, which can take quite some time. I was wondering what the astroquery stance on progressbar utils is? Getting a progressbar would be very easy for this function by simply wrapping the for-loop in astroquery.eso.core.get_headers with the tqdm package and adding an additional keyword, e.g. "progress" to the function to toggle this (that's what I do in my local install). The only code line that needs to be changed is 572:
for dp_id in product_ids -> for dp_id in tqdm(product_ids)

However, this requires adding tqdm as a dependency. I'm sure there are many parts of astroquery that would benefit from having a simple progressbar, but I don't know what astroquery's policy for adding dependencies is.

@bsipocz
Copy link
Member

bsipocz commented Apr 29, 2024

We do use the ProgressBar from astropy at a few places (the main difference is that it handles parallel downloads), so adding a progress bar to ESO is totally fine.

Also, tdqm is quite a small additional dependency, so I would be on board of adding it if you rather use it directly than ProgressBar.

cc @keflavich

@m-samland
Copy link
Author

I wasn't aware of the astropy implementation of ProgressBar. Do you know how it differs from tqdm? Does the astropy implementation work for jupyter notebooks? I'm not hell bend on using tqdm, I just use it because of how easy it is to use, so open to input. I agree that tqdm is not a huge dependency, but I guess we can avoid it if there is precedence in using native astropy for the same functionality.

@keflavich
Copy link
Contributor

I think tqdm is more widely used and more flexible; I'd be happy to switch to it. But functionally, they're the same, and we can just make the progressbar type be specifiable by the user & default to astropy

https://docs.astropy.org/en/stable/api/astropy.utils.console.ProgressBar.html

@bsipocz
Copy link
Member

bsipocz commented Apr 30, 2024

Honestly, I don't know how nicely it works in notebooks or on log files, etc as I'm not using it at a user level.
Do would just reiterate, feel free to choose whichever works better for the module, if it's tqdm then we're happy to add it as a dependency.

(I really don't know what will happen to the astropy version, if it will stay there long term or will be deprecated or maybe just refactored to also use tdqm)

@keflavich
Copy link
Contributor

tqdm works in both with from tqdm.auto import tqdm. I think the astropy version requires you to tell it what environment you're in.

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

No branches or pull requests

3 participants