Explicitly show progress bar for large files #627
Conversation
eta/core/web.py
Outdated
| @@ -168,8 +169,13 @@ def _do_download(self, r, f): | |||
| size_bits = 8 * size_bytes if size_bytes is not None else None | |||
| total_downloaded_bytes = 0 | |||
|
|
|||
| quiet = self.quiet | |||
There was a problem hiding this comment.
When a caller says download_file(url, quiet=True), I think we should treat their request as sacred: there should never be a progress bar.
For context, in FO, there's an fo.config.show_progress_bars setting that can be set to False if a user wants to turn off all progress bars shown throughout the library and all its dependencies (ie invocations of this method). This can actually be of more than just an aesthetic significance: rendering progress bars requires some low-level sorcery like getting the dimensions of the terminal in the caller's environment, which is not possible in all environments and could actually cause hard errors. Obviously having this global flag be fully comprehensive requires that we appropriately pass this setting through to third-party libraries, which we mostly do, but inspecting fiftyone.utils.openimages I see that we haven't respected fo.config.show_progress_bars in this case (either due to laziness on our part and/or because downloading zoo datasets is more of an ad hoc thing, not something that would be built into automations that would happen many times).
All that to say, if we really want to introduce file size-based progress bar control, I think we should add a new syntax. For example:
# Show a progress bar only if file size exceeds 64MB
download_file(url, quiet=64*1024**2)
# Show a progress bar only if file size exceeds our internal default of, say, 128MB
download_file(url, quiet="best")There was a problem hiding this comment.
I do want to note that in the motivating case here of the fiftyone.utils.openimages module, some effort was already put in to show progress bar/logging messages in a bespoke and hopefully helpful way:
quiet=-1: shows a progress bar when downloading the image IDs for (only) the"train"split (since it is much larger): https://github.com/voxel51/fiftyone/blob/a5d2209903d3699be3d310a55ab1e518f858050a/fiftyone/utils/openimages.py#L1864quiet=1: this one suppresses not only the progress bar but also the logging message indicating that a file is being downloaded (only) for the"validation"split (I cannot remember why though!?): https://github.com/voxel51/fiftyone/blob/a5d2209903d3699be3d310a55ab1e518f858050a/fiftyone/utils/openimages.py#L1908
Maybe there's a way to improve user experience just by tweaking the quiet parameters over there?
There was a problem hiding this comment.
I think that makes sense; I was thinking about a case where a user wants to turn off all progress bars but couldn't figure out the exact logic.
I like that we can turn quiet into a more expressive parameter that is also backward compatible:
- true/false (bool): if quiet is true, do not show any progress bar. if quiet is false, show all progress bars. This is the current behavior.
- "best" (str): only shows the progress bar if the file size exceeds certain internal values.
- (int): only show the progress bar if the file size exceeds the value passed in.
- true/false (bool): if quiet is true, do not show progress bar. if quiet is false, show all progress bars. - "best" (str): only show progress bar if the file size exceeds certain internal values. - (int): only show progress bar if the file size exceeds the value passed in.
When following our documentation to download large datasets (such as Open Images), the default behavior is that all progress bars are hidden; hence, sometimes, the snippet appears stuck when downloading large files. This PR changes the behavior so that progress bars are shown explicitly when downloading files greater than a certain size.
Before:
After: