Allow users to disable progress bars

[simonhatcher]

Please add an optional argument to earthaccess.open to disable PQDM progress bars. The progress bars clog up my Jupyter notebook output very badly.

[jhkennedy]

@simonhatcher I agree it's a good idea to provide this control.

For now, since pqdm wraps tqdm, I believe you can set this environment variable to disable the progress bars. You can do this in a shell beforehand:

export TQDM_DISABLE=1

Or in Python ahead of any TQDM imports:

import os

os.environ["TQDM_DISABLE"] = "1"

The environment variable requires tqdm v4.66.0+, but you can also hack it for older versions ahead of any TQDM imports like:

from tqdm import tqdm
from functools import partialmethod

tqdm.__init__ = partialmethod(tqdm.__init__, disable=True)

[simonhatcher]

@jhkennedy Thank you! That does work, but the one challenge is that I'm using TQDM elsewhere and I don't want to globally disable it in the long run.

[jhkennedy]

That does work, but the one challenge is that I'm using TQDM elsewhere and I don't want to globally disable it in the long run.

yes, definitely exactly why we should provide a way to disable it for Earthaccess.

:thinking: I think you could write a context manager that would disable it temporarily... but that's getting to be a lot. Something like (I'm sure there are much cleaner ways to do this):

from tqdm import tqdm
from functools import partialmethod

class DisableTqdm(object):
    def __init__(self):
        self._tqdm_init = tqdm.__init__
        
    def __enter__(self):
        tqdm.__init__ = partialmethod(tqdm.__init__, disable=True)
        
    def __exit__(self, type, value, traceback):
        tqdm.__init__ = self._tqdm_init
        
with DisableTqdm():
    # Do stuff
    ...

[Sherwin-14]

@mfisher87 @jhkennedy I am interested to work on this one. Do you guys have any specific opinions regarding this? I think we can keep two parameters, one for disabling the bars globally and the other one for the current user session. What do you think about this?

[jhkennedy]

@Sherwin-14 that would be great! Thank you!

There's two open issues for tqdm requesting similar functionality upstream that may be relevant:

• https://github.com/tqdm/tqdm/issues/1514
• https://github.com/tqdm/tqdm/issues/1526

With two other context manager suggestions. What are your thoughts on using a context manager for this?

one for disabling the bars globally and the other one for the current user session.

can you describe what you're looking for here? I'm interpreting this to mean (1) "disable it for earthaccess entirely" and (2) provide a way to disable it for specific call.

For (1), are you expecting it to look like:

export EARTHACESS_DISABLE_PROGRESS_BARS=True

and/or

import earthaccess

earthaccess.disable_progress_bars=True

and for (2), are you expecting:

eartaccess.some_method(..., progress=False)

A context manager could also work for 2 like:

with earthaccess.disable_progress:
    eartaccess.some_method(...)

[mfisher87]

I like the idea of having a session setting and a per-invocation setting. Good call!

Maybe controversial, but I feel progress bars should default to off!

Another option is to detect if we're in a notebook by checking with get_ipython().

[Sherwin-14]

can you describe what you're looking for here? I'm interpreting this to mean (1) "disable it for earthaccess entirely" and (2) provide a way to disable it for specific call.

Sorry for being unclear on this. I wanted to use the terminology matt used above (a session setting and a per invocation setting) but I wasn't able to frame it well.

Also what can be the general way forward from here? I am planning to pick out those methods that require tqdm first. Also I agree with the opinion on keeping progress bars off by default (for the methods that use these progress bar) and for the session as well.

[mfisher87]

Also what can be the general way forward from here?

I think you're on the right path. We can add new progress: bool | None = None arguments to top-level methods, and pass it down. If we're building up too many internal parameters that are being passed around, perhaps we can group some related parameters internally-only in a Pydantic model or dataclass.

For implementing the envvar, IMO, we should prioritize what the user passes in as an argument over the envvar, i.e. use envvar only if progress = None (the default), otherwise use the arg the user passes in. If the argument progress is None and no envvar is set, then set progress = False.

All low-level methods would never receive None because we've narrowed to True or False at the top level.

Does that seem reasonable? Would you like this issue assigned to you?

[mfisher87]

Also, would you be interested in triaging permissions, @Sherwin-14 ?

[Sherwin-14]

Also, would you be interested in triaging permissions, @Sherwin-14 ?

Yeah I will be glad to do so :)

[mfisher87]

Thanks for your willingness to help out :heart: :heart: :heart:

Sent the invitation, you can accept it here: https://github.com/nsidc/earthaccess/invitations

You should now be able to assign this issue to yourself if you'd like to test your new permissions :)

[Sherwin-14]

Thanks for your willingness to help out ❤️ ❤️ ❤️

Sent the invitation, you can accept it here: https://github.com/nsidc/earthaccess/invitations

You should now be able to assign this issue to yourself if you'd like to test your new permissions :)

Thanks for the invite ❤️. I will start working on this one asap and if I get stuck somewhere I would reach out to you or @jhkennedy :)

[chuckwondo]

I'm a bit confused as to why this is still open. Issue #792 added the ability for us to pass pqdm_kwargs to top-level functions precisely because of this issue, so we should be able to disable the progress bars by doing something like the following, for example (I have not confirmed that this is correct):

earthaccess.download(..., pqdm_kwargs={"disable": True})

[mfisher87]

Oh yeah! If we feel strongly they should be off by default or support a session-wide setting, though, that will require some additional effort.

[Sherwin-14]

While going through tqdm's documentation, I realised that they do have a disable parameter but I guess it is for suppressing the progressbars for an entire session. If thats the case then we might have to just come up with a different variable for handling per-invocation setting and pass it down all the respective functions that use tqdm alongside the disable parameter.

[chuckwondo]

While going through tqdm's documentation, I realised that they do have a disable parameter but I guess it is for suppressing the progressbars for an entire session. If thats the case then we might have to just come up with a different variable for handling per-invocation setting and pass it down all the respective functions that use tqdm alongside the disable parameter.

As I noted in a previous comment, this approach (or similar, as I have not tested this) should do the trick per invocation:

earthaccess.download(..., pqdm_kwargs={"disable": True})

That's asking pqdm to pass the disable setting down to tqdm.

[Sherwin-14]

While going through tqdm's documentation, I realised that they do have a disable parameter but I guess it is for suppressing the progressbars for an entire session. If thats the case then we might have to just come up with a different variable for handling per-invocation setting and pass it down all the respective functions that use tqdm alongside the disable parameter.

As I noted in a previous comment, this approach (or similar, as I have not tested this) should do the trick per invocation:

earthaccess.download(..., pqdm_kwargs={"disable": True})

That's asking pqdm to pass the disable setting down to tqdm.

Thanks for clarifying this! I had this confusion because of the word "entire" written in the tqdm doc, I thought entire might mean a session setting.

[itcarroll]

Am I correct that the suggestion to disable progress bars by default, noted as possibly controversial by @mfisher87, has now been merged but not released?

I think many users will think something has gone wrong when they don't see progress bars. At the least, I don't think this should be released without a global|session setting in place to enable progress bars.

[itcarroll]

Also, the pqdm_kwargs option does not work for earthaccess.download when in region.

[chuckwondo]

Am I correct that the suggestion to disable progress bars by default, noted as possibly controversial by @mfisher87, has now been merged but not released?

I think many users will think something has gone wrong when they don't see progress bars. At the least, I don't think this should be released without a global|session setting in place to enable progress bars.

Yes, the default has been changed to not show progress bars (not yet released). While I personally prefer that default, I agree that this might cause confusion for users who have become accustomed to seeing them by default, so I support adjusting the recent change (which adds the ability to disable them) to keep them enabled by default.

Regarding some sort of "global" or "session" settings, I also agree that we need to design a means of supporting this. I think this may be worthy of opening a design discussion. I see at least 2 alternatives off the top of my head, so I'm going to open a discussion for it.

Also, the pqdm_kwargs option does not work for earthaccess.download when in region.

Right. If I'm reading the code correctly, this is because "in region" downloading does not (yet) make use of pqdm at all, but there is a comment about the need to add support for concurrency, so this is perhaps worthy of a new issue (if one doesn't already exist).

[chuckwondo]

See https://github.com/nsidc/earthaccess/discussions/1004

[itcarroll]

Opportunity to revisit #761. I will aim to make it to next hackday and attempt progress there with this fairly simple decision as a practical case.

[betolink]

I'm working on a PR to fix various issues with the download/open methods and as part of it I think I ran into this behavior of nor showing the progress bar, I think the default should be to show it and we should only include an easy way of disabling it. It is very obscure that the cell is being executing and a user has no idea of the download progress.

What are your thoughts? @chuckwondo @Sherwin-14 @itcarroll

[mfisher87]

Running earthaccess in a script and in a notebook are very different cases, and I could see a progress bar's presence or absence being disruptive in each. It's very common for programs to detect whether they're in an interactive context and change their defaults based on this. I think we should continue to provide an override so the user is always in control, and change our default to show progress in interactive sessions, and not show progress in non-interactive sessions. One way we can do that is checking IPython.get_ipython(). What about something like (untested):

def is_interactive() -> bool:
    """Detect if earthaccess is being used in an interactive session.

    Interactive sessions include Jupyter Notebooks, IPython REPL, and default Python REPL.
    """
    try:
        from IPython import get_ipython
        
        # IPython Notebook or REPL:
        if get_ipython() is not None:
            return True
    except ImportError:
        pass

    # Python REPL
    if hasattr(sys, 'ps1'):
        return True

    return False

[betolink]

Agree, I can add this on the PR and the default will be to show the progress bar if is_interactive is True. I don't know if this needs to be a public method. Maybe _is_interactive()?

[mfisher87]

Agreed, private method :)

[mfisher87]

And we should update our docs to describe this functionality. Looks like we missed it in #988 !

[betolink]

Working on the docs and notebook tomorrow, here is the draft PR #1061

[asteiker]

Closing as complete with #988