From de9d3a035e8f26a5e9e1f22bab5d8c26cfe97db6 Mon Sep 17 00:00:00 2001
From: Orion Yeung <11580988+orionyeung001@users.noreply.github.com>
Date: Sat, 25 Nov 2023 21:52:22 -0600
Subject: [PATCH 1/8] add --field-prefix via click.option and update doc

added option proposed in scikit-hep/histoprint #121

implement behavior of --field-prefix option

uses click callback on the field option to handle this.

updated docs to include help text for --field-prefix option

remove type annotations

One of these type annotations was actually incorrect, but I noticed
that they weren't used in this file. Opted to remove instead of fix
---
 README.rst        |  6 ++++++
 histoprint/cli.py | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/README.rst b/README.rst
index 8334a79..93ab3f8 100644
--- a/README.rst
+++ b/README.rst
@@ -179,6 +179,12 @@ It can read in files or take data directly from STDIN::
                                       'tree/branch[:,2]' to histogram the 3rd
                                       elements of a vector-like branch.
 
+      --field-prefix TEXT             String to prepend to each argument passed to
+                                      --field option following --field-prefix E.g.
+                                      'histoprint -f Muon_eta --field-prefix Tau_
+                                      -f eta -f phi' ... would refer to fields
+                                      'Muon_eta', 'Tau_eta' and 'Tau_phi''
+
       -C, --cut TEXT                  Filter the data to be plotted by a cut
                                       condition. For ROOT files, variables must be
                                       referenced by their branch name within the
diff --git a/histoprint/cli.py b/histoprint/cli.py
index a5e7e39..e7ec3c8 100644
--- a/histoprint/cli.py
+++ b/histoprint/cli.py
@@ -9,6 +9,11 @@
 import histoprint as hp
 import histoprint.formatter as formatter
 
+def fieldprefixer_callback(ctx, param, fields):
+    if 'field_prefix' in ctx.params:
+        fields = map(lambda field: ctx.params['field_prefix'] + field, fields)
+    return tuple(fields)
+
 
 @click.command()
 @click.argument("infile", type=click.Path(exists=True, dir_okay=False, allow_dash=True))
@@ -74,6 +79,16 @@
     "single TH1, or one or more paths to TTree branches. Also supports slicing "
     "of array-like branches, e.g. use 'tree/branch[:,2]' to histogram the 3rd "
     "elements of a vector-like branch.",
+    callback=fieldprefixer_callback
+)
+@click.option(
+    "--field-prefix",
+    "field_prefix",
+    type=str,
+    multiple=False,
+    help="String to prepend to each argument passed to --field option following --field-prefix"
+    "\nE.g. `histoprint -f Muon_eta --field-prefix Tau_ -f eta -f phi` ... would refer to fields"
+    "\n\t`Muon_eta`, `Tau_eta` and `Tau_phi`'",
 )
 @click.option(
     "-C",
@@ -109,6 +124,7 @@ def histoprint(infile, **kwargs):
 
     INFILE can be '-', in which case the data is read from STDIN.
     """
+    del kwargs["field_prefix"]
 
     # Read file into buffer for use by implementations
     try:
@@ -148,7 +164,6 @@ def histoprint(infile, **kwargs):
     click.echo("Could not interpret file format.", err=True)
     exit(1)
 
-
 def _bin_edges(kwargs, data):
     """Get the desired bin edges."""
     bins = kwargs.pop("bins", "10")

From dbeda01328db934985335deb18c99c88ec09ac09 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Mon, 27 Nov 2023 17:15:18 +0000
Subject: [PATCH 2/8] [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
---
 histoprint/cli.py | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/histoprint/cli.py b/histoprint/cli.py
index e7ec3c8..448d1e6 100644
--- a/histoprint/cli.py
+++ b/histoprint/cli.py
@@ -9,9 +9,10 @@
 import histoprint as hp
 import histoprint.formatter as formatter
 
+
 def fieldprefixer_callback(ctx, param, fields):
-    if 'field_prefix' in ctx.params:
-        fields = map(lambda field: ctx.params['field_prefix'] + field, fields)
+    if "field_prefix" in ctx.params:
+        fields = map(lambda field: ctx.params["field_prefix"] + field, fields)
     return tuple(fields)
 
 
@@ -79,7 +80,7 @@ def fieldprefixer_callback(ctx, param, fields):
     "single TH1, or one or more paths to TTree branches. Also supports slicing "
     "of array-like branches, e.g. use 'tree/branch[:,2]' to histogram the 3rd "
     "elements of a vector-like branch.",
-    callback=fieldprefixer_callback
+    callback=fieldprefixer_callback,
 )
 @click.option(
     "--field-prefix",
@@ -164,6 +165,7 @@ def histoprint(infile, **kwargs):
     click.echo("Could not interpret file format.", err=True)
     exit(1)
 
+
 def _bin_edges(kwargs, data):
     """Get the desired bin edges."""
     bins = kwargs.pop("bins", "10")

From 76fcd3f33840c596d4bbca8fd5822596e32601cd Mon Sep 17 00:00:00 2001
From: Orion Yeung <11580988+orionyeung001@users.noreply.github.com>
Date: Tue, 28 Nov 2023 11:24:41 -0600
Subject: [PATCH 3/8] add cli test to github `test' workflow

---
 .github/workflows/pythontests.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.github/workflows/pythontests.yml b/.github/workflows/pythontests.yml
index 9ac6546..1df2960 100644
--- a/.github/workflows/pythontests.yml
+++ b/.github/workflows/pythontests.yml
@@ -40,6 +40,7 @@ jobs:
         wget --quiet http://scikit-hep.org/uproot3/examples/Event.root
         histoprint -s Event.root -f T/event/fTracks/fTracks.fYfirst -f T/event/fTracks/fTracks.fYlast[:,0]
         histoprint -s Event.root -f T/fTracks.fYfirst -f T/event/fTracks/fTracks.fYlast[:,0] -C 'fNtrack > 5'
+        histoprint -s Event.root --field-prefix T/event/fTracks/fTracks. -f fYfirst -f fYlast[:,0]
         histoprint -s Event.root -f htime
 
   pre-commit:

From b2bac5990a6d41756f2f78373721d2a173abcc15 Mon Sep 17 00:00:00 2001
From: Orion Yeung <11580988+orionyeung001@users.noreply.github.com>
Date: Tue, 28 Nov 2023 11:26:37 -0600
Subject: [PATCH 4/8] add equivalence test for --field-prefix

---
 tests/test_basic.py | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tests/test_basic.py b/tests/test_basic.py
index 4828212..0402032 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -181,3 +181,30 @@ def test_rich_histogram():
     tab.add_row(hist, Align.center(hist), Align.right(hist))
 
     rich.print(tab)
+
+def cli_prefix_equivalence_test():
+    """
+    verify that the --field-prefix behaves exactly as if
+    using full name of field in --field
+    """
+    from click.testing import CliRunner
+    from histoprint.cli import histoprint as cli
+
+    runner = CliRunner()
+    res = runner.invoke(cli, [
+                        "-s", "tests/data/histograms.root",
+                        "-f", "two",
+                        "-f", "three",
+                        ])
+    assert res.exit_code == 0
+
+    res_prefixed = runner.invoke(cli, [
+                        "-s", "tests/data/histograms.root",
+                        "-f", "wo",
+                        "--field-prefix", "t",
+                        "-f", "hree",
+                        ])
+    assert res_prefixed.exit_code == 0
+
+    # assert equivalent output
+    assert res.output == res_prefixed.output

From a1d0ee2c106920096e786511c2ab4b8de11eee4d Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Tue, 28 Nov 2023 17:28:39 +0000
Subject: [PATCH 5/8] [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
---
 tests/test_basic.py | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/tests/test_basic.py b/tests/test_basic.py
index 0402032..3dd3797 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -182,28 +182,43 @@ def test_rich_histogram():
 
     rich.print(tab)
 
+
 def cli_prefix_equivalence_test():
     """
     verify that the --field-prefix behaves exactly as if
     using full name of field in --field
     """
     from click.testing import CliRunner
+
     from histoprint.cli import histoprint as cli
 
     runner = CliRunner()
-    res = runner.invoke(cli, [
-                        "-s", "tests/data/histograms.root",
-                        "-f", "two",
-                        "-f", "three",
-                        ])
+    res = runner.invoke(
+        cli,
+        [
+            "-s",
+            "tests/data/histograms.root",
+            "-f",
+            "two",
+            "-f",
+            "three",
+        ],
+    )
     assert res.exit_code == 0
 
-    res_prefixed = runner.invoke(cli, [
-                        "-s", "tests/data/histograms.root",
-                        "-f", "wo",
-                        "--field-prefix", "t",
-                        "-f", "hree",
-                        ])
+    res_prefixed = runner.invoke(
+        cli,
+        [
+            "-s",
+            "tests/data/histograms.root",
+            "-f",
+            "wo",
+            "--field-prefix",
+            "t",
+            "-f",
+            "hree",
+        ],
+    )
     assert res_prefixed.exit_code == 0
 
     # assert equivalent output

From e5956bdb576be8bed417874e2ceae080160b7f3a Mon Sep 17 00:00:00 2001
From: Orion Yeung <11580988+orionyeung001@users.noreply.github.com>
Date: Tue, 28 Nov 2023 12:50:07 -0600
Subject: [PATCH 6/8] fixes from PR #124 feedback

---
 histoprint/cli.py   |  5 ++---
 tests/test_basic.py | 25 +++++++++++--------------
 2 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/histoprint/cli.py b/histoprint/cli.py
index 448d1e6..e9e60d3 100644
--- a/histoprint/cli.py
+++ b/histoprint/cli.py
@@ -87,9 +87,8 @@ def fieldprefixer_callback(ctx, param, fields):
     "field_prefix",
     type=str,
     multiple=False,
-    help="String to prepend to each argument passed to --field option following --field-prefix"
-    "\nE.g. `histoprint -f Muon_eta --field-prefix Tau_ -f eta -f phi` ... would refer to fields"
-    "\n\t`Muon_eta`, `Tau_eta` and `Tau_phi`'",
+    help="String to prepend to all values indicated with --field option, "
+    "ignores position where this and --field options are specified.",
 )
 @click.option(
     "-C",
diff --git a/tests/test_basic.py b/tests/test_basic.py
index 3dd3797..c808389 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -3,7 +3,9 @@
 import numpy as np
 import pytest
 from uhi.numpy_plottable import ensure_plottable_histogram
+from click.testing import CliRunner
 
+from histoprint.cli import histoprint as cli
 import histoprint as hp
 
 
@@ -183,16 +185,12 @@ def test_rich_histogram():
     rich.print(tab)
 
 
-def cli_prefix_equivalence_test():
-    """
-    verify that the --field-prefix behaves exactly as if
-    using full name of field in --field
-    """
-    from click.testing import CliRunner
-
-    from histoprint.cli import histoprint as cli
+def test_cli_opt_field_prefix():
+    """Test equivalence of --field-prefix cli option to explicit prepend on --field."""
 
     runner = CliRunner()
+
+    # tests for fields "two" and "three" since share prefix of "t"
     res = runner.invoke(
         cli,
         [
@@ -204,8 +202,6 @@ def cli_prefix_equivalence_test():
             "three",
         ],
     )
-    assert res.exit_code == 0
-
     res_prefixed = runner.invoke(
         cli,
         [
@@ -213,13 +209,14 @@ def cli_prefix_equivalence_test():
             "tests/data/histograms.root",
             "-f",
             "wo",
-            "--field-prefix",
-            "t",
             "-f",
             "hree",
+            "--field-prefix",
+            "t",
         ],
     )
-    assert res_prefixed.exit_code == 0
 
-    # assert equivalent output
+    # assert succesful equivalent output
+    assert res.exit_code == 0
+    assert res_prefixed.exit_code == 0
     assert res.output == res_prefixed.output

From 944648a5151e9668cecde5e66cbca36d0aa04fa4 Mon Sep 17 00:00:00 2001
From: "pre-commit-ci[bot]"
 <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Date: Tue, 28 Nov 2023 18:51:19 +0000
Subject: [PATCH 7/8] [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci
---
 tests/test_basic.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test_basic.py b/tests/test_basic.py
index c808389..2b8ac9f 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -2,11 +2,11 @@
 
 import numpy as np
 import pytest
-from uhi.numpy_plottable import ensure_plottable_histogram
 from click.testing import CliRunner
+from uhi.numpy_plottable import ensure_plottable_histogram
 
-from histoprint.cli import histoprint as cli
 import histoprint as hp
+from histoprint.cli import histoprint as cli
 
 
 def test_hist():

From c6633c19115426b5a37716dfb4c53cf8d4546401 Mon Sep 17 00:00:00 2001
From: Orion Yeung <11580988+orionyeung001@users.noreply.github.com>
Date: Wed, 29 Nov 2023 19:11:13 -0600
Subject: [PATCH 8/8] setup testing for --field-prefix option

migrate prefixing logic from callback into main
add test case and data
update changelog
---
 CHANGELOG.md                 |  5 ++++-
 histoprint/cli.py            | 10 ++--------
 tests/data/2D-prefixable.csv | 10 ++++++++++
 tests/test_basic.py          | 15 +++++++--------
 4 files changed, 23 insertions(+), 17 deletions(-)
 create mode 100644 tests/data/2D-prefixable.csv

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0567917..c90896f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,9 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+- add support for --field-prefix option at cli
+
 ## [2.4.0]
 
-### Addded
+### Added
 - RichHistogram class for use with the `rich` package.
 - Argument to set maximum count in bins for plotting.
 
diff --git a/histoprint/cli.py b/histoprint/cli.py
index e9e60d3..2895f40 100644
--- a/histoprint/cli.py
+++ b/histoprint/cli.py
@@ -10,12 +10,6 @@
 import histoprint.formatter as formatter
 
 
-def fieldprefixer_callback(ctx, param, fields):
-    if "field_prefix" in ctx.params:
-        fields = map(lambda field: ctx.params["field_prefix"] + field, fields)
-    return tuple(fields)
-
-
 @click.command()
 @click.argument("infile", type=click.Path(exists=True, dir_okay=False, allow_dash=True))
 @click.option(
@@ -80,7 +74,6 @@ def fieldprefixer_callback(ctx, param, fields):
     "single TH1, or one or more paths to TTree branches. Also supports slicing "
     "of array-like branches, e.g. use 'tree/branch[:,2]' to histogram the 3rd "
     "elements of a vector-like branch.",
-    callback=fieldprefixer_callback,
 )
 @click.option(
     "--field-prefix",
@@ -124,7 +117,8 @@ def histoprint(infile, **kwargs):
 
     INFILE can be '-', in which case the data is read from STDIN.
     """
-    del kwargs["field_prefix"]
+    if (prefix := kwargs.pop("field_prefix", None)) is not None:
+        kwargs["fields"] = map(lambda s: prefix + s, kwargs["fields"])
 
     # Read file into buffer for use by implementations
     try:
diff --git a/tests/data/2D-prefixable.csv b/tests/data/2D-prefixable.csv
new file mode 100644
index 0000000..dae7a37
--- /dev/null
+++ b/tests/data/2D-prefixable.csv
@@ -0,0 +1,10 @@
+Txx,Txy
+1,4
+2,5
+2,5
+3,6
+3,nan
+3,6
+4,7
+4,7
+5,8
diff --git a/tests/test_basic.py b/tests/test_basic.py
index 2b8ac9f..9ce8095 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -190,29 +190,28 @@ def test_cli_opt_field_prefix():
 
     runner = CliRunner()
 
-    # tests for fields "two" and "three" since share prefix of "t"
     res = runner.invoke(
         cli,
         [
             "-s",
-            "tests/data/histograms.root",
+            "tests/data/2D-prefixable.csv",
             "-f",
-            "two",
+            "Txx",
             "-f",
-            "three",
+            "Txy",
         ],
     )
     res_prefixed = runner.invoke(
         cli,
         [
             "-s",
-            "tests/data/histograms.root",
+            "tests/data/2D-prefixable.csv",
             "-f",
-            "wo",
+            "x",
             "-f",
-            "hree",
+            "y",
             "--field-prefix",
-            "t",
+            "Tx",
         ],
     )