From 2eefbb5281a0bdc2d7763d5de4ef81b38c70300e Mon Sep 17 00:00:00 2001
From: "Josef M. Gallmetzer" <64498081+galjos@users.noreply.github.com>
Date: Mon, 24 Aug 2026 09:22:38 +0200
Subject: [PATCH] fix: make --progress flag match its help text

The --progress option was registered with store_false, so passing
it disabled the progress bar although its help reads "Show progress
bar." It now uses BooleanOptionalAction: --progress shows the bar,
--no-progress hides it, and the default stays enabled.

The RDF module also copied config.with_progress_bar into its
namespace at import time, before the CLI parses arguments, so the
flag never reached the two tqdm loops in the analysis. It now reads
the config attribute at call time like msd, vacf and momentum do.
---
 PQAnalysis/analysis/rdf/rdf.py     |  6 +++---
 PQAnalysis/cli/_argument_parser.py |  5 ++++-
 tests/analysis/rdf/test_rdf.py     | 28 ++++++++++++++++++++++++++
 tests/cli/test_main.py             | 32 ++++++++++++++++++++++++++++++
 4 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/PQAnalysis/analysis/rdf/rdf.py b/PQAnalysis/analysis/rdf/rdf.py
index ff8e37d1..129538db 100644
--- a/PQAnalysis/analysis/rdf/rdf.py
+++ b/PQAnalysis/analysis/rdf/rdf.py
@@ -21,7 +21,7 @@
 from PQAnalysis.utils.progress import tqdm
 
 # local absolute imports
-from PQAnalysis.config import with_progress_bar
+from PQAnalysis import config
 from PQAnalysis.types import Np1DNumberArray, PositiveInt, PositiveReal
 from PQAnalysis.core import distance, Cell, Cells
 from PQAnalysis.traj import (
@@ -873,7 +873,7 @@ def _calculate_bins(self):
         for frame in tqdm(
             itertools.chain([self.first_frame], self.frame_generator),
             total=self.n_frames,
-            disable=not with_progress_bar
+            disable=not config.with_progress_bar
         ):
             for i, reference_index in enumerate(self.reference_indices):
 
@@ -953,7 +953,7 @@ def _calculate_bins_raw(self):
         for values, cell in tqdm(
             self._raw_reader.raw_frame_generator(),
             total=self.n_frames,
-            disable=not with_progress_bar):
+            disable=not config.with_progress_bar):
 
             counter += 1
 
diff --git a/PQAnalysis/cli/_argument_parser.py b/PQAnalysis/cli/_argument_parser.py
index bb26ec76..7eb924e2 100644
--- a/PQAnalysis/cli/_argument_parser.py
+++ b/PQAnalysis/cli/_argument_parser.py
@@ -195,7 +195,10 @@ def _parse_progress(self):
         The progress argument is an optional argument and defaults to True.
         """
         super().add_argument(
-            '--progress', action='store_false', help='Show progress bar.'
+            '--progress',
+            action=argparse.BooleanOptionalAction,
+            default=True,
+            help='Show progress bar.'
         )
 
     def _parse_version(self):
diff --git a/tests/analysis/rdf/test_rdf.py b/tests/analysis/rdf/test_rdf.py
index 09706041..0b7c0ae3 100644
--- a/tests/analysis/rdf/test_rdf.py
+++ b/tests/analysis/rdf/test_rdf.py
@@ -1,6 +1,9 @@
+import sys
+
 import numpy as np
 import pytest
 
+from PQAnalysis import config
 from PQAnalysis.analysis.rdf.exceptions import RDFError
 from PQAnalysis.analysis import RDF
 from PQAnalysis.traj import Trajectory
@@ -298,6 +301,31 @@ def test__add_to_bins():
 
 
 
+def test_progress_bar_binds_config_at_call_time(monkeypatch):
+    # config.with_progress_bar is set by the CLI after the module
+    # import, so it must be read at call time, not bound by value
+    # at import time
+    captured = {}
+
+    def fake_tqdm(iterable, **kwargs):
+        captured.update(kwargs)
+        return iterable
+
+    rdf_module = sys.modules[RDF.__module__]
+    monkeypatch.setattr(rdf_module, "tqdm", fake_tqdm)
+
+    monkeypatch.setattr(config, "with_progress_bar", False)
+    RDF(_make_no_intra_trajectory(), ["H"], ["C"], delta_r=0.5, n_bins=5).run()
+    assert captured["disable"] is True
+
+    captured.clear()
+
+    monkeypatch.setattr(config, "with_progress_bar", True)
+    RDF(_make_no_intra_trajectory(), ["H"], ["C"], delta_r=0.5, n_bins=5).run()
+    assert captured["disable"] is False
+
+
+
 class TestRDF:
 
     def test__init__type_checking(self, caplog):
diff --git a/tests/cli/test_main.py b/tests/cli/test_main.py
index 224b4acd..618141a4 100644
--- a/tests/cli/test_main.py
+++ b/tests/cli/test_main.py
@@ -78,3 +78,35 @@ def test_argcomplete_is_loaded_only_for_shell_completion(monkeypatch):
 
     assert calls == [parser]
     assert args.progress is True
+
+
+
+@pytest.mark.parametrize(
+    ("flags", "expected"),
+    [
+        ([], True),
+        (["--progress"], True),
+        (["--no-progress"], False),
+    ],
+)
+def test_progress_flag_matches_its_help_text(monkeypatch, flags, expected):
+    # --progress used to be a store_false flag, so passing it hid the
+    # progress bar although its help text reads "Show progress bar."
+    monkeypatch.setattr(argument_parser, "print_header", lambda: None)
+    monkeypatch.setattr(
+        argument_parser.config,
+        "with_progress_bar",
+        argument_parser.config.with_progress_bar,
+    )
+
+    parser = argument_parser._ArgumentParser(prog="pqanalysis-test")
+    root_logger = argument_parser.logging.getLogger()
+    original_level = root_logger.level
+
+    try:
+        args = parser.parse_args([*flags, "--log-file", "off"])
+    finally:
+        root_logger.setLevel(original_level)
+
+    assert args.progress is expected
+    assert argument_parser.config.with_progress_bar is expected