Add ability to turn on and off progress bars

[sgugger]

Not everyone wants to see progress bars when downloading models/datasets as tqdm can clog the logs pretty easily. That's why we have a mode in Transformers to deactivate tqdm when setting some env variable using these utils.

Currently, Transformers enforces huggingface_hub uses this tqdm class by calling hf_hub_download under a contextmanager that monkey-patches huggingface_hub.file_download.tqdm. It would be much nicer if we could rely on any solution huggingface_hub implements instead :-)

[Wauplin]

Hi @sgugger, thanks for creating this issue. Seems legitimate to do this in huggingface_hub. From an API point of view, how would you see it ?

From a usability perspective, how would you prefer it ?

Either directly via a HF_HUB_DISABLE_PROGRESS_BARS env variable ? If you already have an env variable for it in transformers, we should align on the name.

Either have an helper to globally disable tqdm ? For example:

def disable_progress_bars() -> None:
   (...)

def enable_progress_bars() -> None:
   (...)

Or have a context manager to temporary disable tqdm ? Something like:

def some_function_in_transformers(...):
    (...)
    with huggingface_hub.no_progress_bars():
        hf_hub_download(my_file)

Implementation-wise, the EmptyTqdm class seems nice but we would need to adapt it for huggingface_hub to have some.close() or .update() methods used here or here. Otherwise, what do you think about the proposition mentioned in this comment to replace the EmptyTqdm ? Do I miss a use case that is not handled by it ?

[sgugger]

I think offering options 1 and 2 would be nice: the env variable for individual users, and the helpers for libraries using huggingface_hub like transformers (I can patch an env variable from Transformers but that does not feel clean ;-) )

The context manager seems less useful as I feel users will either want progress bars all the time or never. I used this solution when monkey-patching huggingface_hub just to make sure the monkey-patch was properly undone.

The suggestion in the forcing disable in a subclass to replace EmptyTqdm works well I believe.

[Wauplin]

Thanks for the feedback :) Seems reasonable. Will try to make a PR about it soon

[LysandreJik]

Closed by https://github.com/huggingface/huggingface_hub/pull/987