From bc5cf85b8a03a03db4898551dbf558ad8ef5452e Mon Sep 17 00:00:00 2001
From: Matthew Feickert <matthew.feickert@cern.ch>
Date: Fri, 23 Sep 2022 16:38:05 -0500
Subject: [PATCH] feat: Make `par_names` a _ModelConfig property attribute
 (#2027)

* Make pyhf.pdf._ModelConfig.par_names() property attribute pyhf.pdf._ModelConfig.par_names,
  like the the pyhf.pdf._ModelConfig.par_order API.
* Update all usage of `.par_names()` to `par_names`.
* Use unittest.mock.PropertyMock to properly mock par_names return.
---
 src/pyhf/optimize/mixins.py |  2 +-
 src/pyhf/pdf.py             |  5 ++++-
 tests/test_optim.py         | 12 ++++++++----
 tests/test_pdf.py           |  4 ++--
 tests/test_simplemodels.py  |  8 ++++----
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/pyhf/optimize/mixins.py b/src/pyhf/optimize/mixins.py
index c9c24f663c..43580d9b92 100644
--- a/src/pyhf/optimize/mixins.py
+++ b/src/pyhf/optimize/mixins.py
@@ -181,7 +181,7 @@ def minimize(
 
         # handle non-pyhf ModelConfigs
         try:
-            par_names = pdf.config.par_names()
+            par_names = pdf.config.par_names
         except AttributeError:
             par_names = None
 
diff --git a/src/pyhf/pdf.py b/src/pyhf/pdf.py
index 99923afeb1..8c7420a9ce 100644
--- a/src/pyhf/pdf.py
+++ b/src/pyhf/pdf.py
@@ -358,6 +358,7 @@ def par_slice(self, name):
         """
         return self.par_map[name]['slice']
 
+    @property
     def par_names(self):
         """
         The names of the parameters in the model including binned-parameter indexing.
@@ -370,8 +371,10 @@ def par_names(self):
             >>> model = pyhf.simplemodels.uncorrelated_background(
             ...     signal=[12.0, 11.0], bkg=[50.0, 52.0], bkg_uncertainty=[3.0, 7.0]
             ... )
-            >>> model.config.par_names()
+            >>> model.config.par_names
             ['mu', 'uncorr_bkguncrt[0]', 'uncorr_bkguncrt[1]']
+
+        .. versionchanged:: 0.7.0 Changed from method to property attribute.
         """
         _names = []
         for name in self.par_order:
diff --git a/tests/test_optim.py b/tests/test_optim.py
index 18b8b9d4b9..cc245833d0 100644
--- a/tests/test_optim.py
+++ b/tests/test_optim.py
@@ -1,3 +1,4 @@
+from unittest.mock import patch, PropertyMock
 import pyhf
 from pyhf.optimize.mixins import OptimizerMixin
 from pyhf.optimize.common import _get_tensor_shim, _make_stitch_pars
@@ -576,7 +577,10 @@ def test_minuit_param_names(mocker):
     assert 'minuit' in result
     assert result.minuit.parameters == ('mu', 'uncorr_bkguncrt[0]')
 
-    pdf.config.par_names = mocker.Mock(return_value=None)
-    _, result = pyhf.infer.mle.fit(data, pdf, return_result_obj=True)
-    assert 'minuit' in result
-    assert result.minuit.parameters == ('x0', 'x1')
+    with patch(
+        "pyhf.pdf._ModelConfig.par_names", new_callable=PropertyMock
+    ) as mock_par_names:
+        mock_par_names.return_value = None
+        _, result = pyhf.infer.mle.fit(data, pdf, return_result_obj=True)
+        assert "minuit" in result
+        assert result.minuit.parameters == ("x0", "x1")
diff --git a/tests/test_pdf.py b/tests/test_pdf.py
index 948aa1c0f2..045575d725 100644
--- a/tests/test_pdf.py
+++ b/tests/test_pdf.py
@@ -957,7 +957,7 @@ def test_par_names_scalar_nonscalar():
 
     model = pyhf.Model(spec, poi_name="scalar")
     assert model.config.par_order == ["scalar", "nonscalar"]
-    assert model.config.par_names() == [
+    assert model.config.par_names == [
         'scalar',
         'nonscalar[0]',
     ]
@@ -1159,7 +1159,7 @@ def test_pdf_clipping(backend):
     model = ws.model()
     data = tensorlib.astensor([100.0, 100.0, 10.0, 0.0, 0.0])
 
-    for par_name in model.config.par_names():
+    for par_name in model.config.par_names:
         if "np" in par_name:
             par_values.append(-0.6)  # np_1 / np_2
         else:
diff --git a/tests/test_simplemodels.py b/tests/test_simplemodels.py
index 3ff8b75beb..b7b722a156 100644
--- a/tests/test_simplemodels.py
+++ b/tests/test_simplemodels.py
@@ -18,7 +18,7 @@ def test_correlated_background(backend):
     assert model.config.channels == ["single_channel"]
     assert model.config.samples == ["background", "signal"]
     assert model.config.par_order == ["correlated_bkg_uncertainty", "mu"]
-    assert model.config.par_names() == ['correlated_bkg_uncertainty', "mu"]
+    assert model.config.par_names == ["correlated_bkg_uncertainty", "mu"]
     assert model.config.suggested_init() == [0.0, 1.0]
 
 
@@ -29,7 +29,7 @@ def test_uncorrelated_background(backend):
     assert model.config.channels == ["singlechannel"]
     assert model.config.samples == ["background", "signal"]
     assert model.config.par_order == ["mu", "uncorr_bkguncrt"]
-    assert model.config.par_names() == [
+    assert model.config.par_names == [
         'mu',
         'uncorr_bkguncrt[0]',
         'uncorr_bkguncrt[1]',
@@ -52,7 +52,7 @@ def test_correlated_background_default_backend(default_backend):
     assert model.config.channels == ["single_channel"]
     assert model.config.samples == ["background", "signal"]
     assert model.config.par_order == ["correlated_bkg_uncertainty", "mu"]
-    assert model.config.par_names() == ['correlated_bkg_uncertainty', "mu"]
+    assert model.config.par_names == ["correlated_bkg_uncertainty", "mu"]
     assert model.config.suggested_init() == [0.0, 1.0]
 
 
@@ -68,7 +68,7 @@ def test_uncorrelated_background_default_backend(default_backend):
     assert model.config.channels == ["singlechannel"]
     assert model.config.samples == ["background", "signal"]
     assert model.config.par_order == ["mu", "uncorr_bkguncrt"]
-    assert model.config.par_names() == [
+    assert model.config.par_names == [
         'mu',
         'uncorr_bkguncrt[0]',
         'uncorr_bkguncrt[1]',