Set disable=None by default for progress bar in ctapipe tools

[morcuended]

Sets tqdm(..., disable=None) in process and merge ctapipe tools.

By default the progress bar is displayed when running the tools in interactive mode. If they are executed in non-tty mode the progress bar is disabled. There is also a command line option --disable-progress to disable it.

I added some tests to check the expected behavior in both tty and non-tty modes.

Closes #2366

[kosack]

You need to remove the --progress option in a few other tests in test_process.py, I think.

Another potential suggestion (optional): since many people may still have this option in their scripts, maybe deprecating it more gracefully would be nice? e.g. add a --progress option that does nothing except emit a warning that the --progress option is deprecated and no longer needed (and to use --disable-progress to remove the automatic progress bar if necessary). We might want to always do that on changes to top-level command-line options.

[morcuended]

You need to remove the --progress option in a few other tests in test_process.py, I think.

Yes, done. Thanks.

Another potential suggestion (optional): since many people may still have this option in their scripts, maybe deprecating it more gracefully would be nice? e.g. add a --progress option that does nothing except emit a warning that the --progress option is deprecated and no longer needed (and to use --disable-progress to remove the automatic progress bar if necessary). We might want to always do that on changes to top-level command-line options.

Thanks, Good idea. I will do so.

[morcuended]

I tried to implement some tests for disabling the progress bar when using non-tty mode and also that it is displayed automatically in tty mode. But I found no straightforward way to capture the progress bar using, for example, subprocess. If you think tests on this are anyways necessary, we could try to find a way.

[maxnoe]

I think the flag definition should be left as before, but the action needs to changed accordingly.

I think the desired behavior is:

• Flag not given sets disable=None, so tqdm checks if it runs in an interactive environment or not
• --progress forces the progress bar -> disable=False
• --no-progress forces the progress bar to be disabled -> disable=True.

This would be the case for:

"progress": ({"ApplyModels": {"disable_progress_bar": False}}, "Always enable progress bar."),
"no-progress": ({"ApplyModels": {"disable_progress_bar": True}}, "Always disable progress bar."),

[morcuended]

Thanks for the suggestion, indeed that would not change the current implementation. It is actually what we briefly discussed in the meeting, I think.

[maxnoe]

Looking at all this duplicated code now, we could move these options to the base Tool class with a method progress_bar so we have this only in one place and the implementations can just do:

for foo in self.progress_bar(iterable, **options):
    ...