Skip to content

Commit

Permalink
feat: Make par_names a _ModelConfig property attribute (#2027)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
matthewfeickert authored Sep 23, 2022
1 parent cccbda1 commit bc5cf85
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/pyhf/optimize/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
5 changes: 4 additions & 1 deletion src/pyhf/pdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
12 changes: 8 additions & 4 deletions tests/test_optim.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")
4 changes: 2 additions & 2 deletions tests/test_pdf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]',
]
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions tests/test_simplemodels.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]


Expand All @@ -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]',
Expand All @@ -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]


Expand All @@ -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]',
Expand Down

0 comments on commit bc5cf85

Please sign in to comment.