fix: Ensure _ModelConfig.suggested_fixed list contains only booleans for all modifiers

[alexander-held]

Description

The staterror refactoring in #1639 caused model.config.suggested_fixed() to return numpy.bool_ elements for staterror modifiers. Those behave differently from bool objects, and for consistency I believe it is useful to have a consistent return type, in line with the documented return type of List[bool]:

pyhf/src/pyhf/pdf.py

Lines 349 to 355 in f25b8dd

	def suggested_fixed(self):
	"""
	Identify the fixed parameters in the model.
	Returns:
	:obj:`list`: A list of booleans, ``True`` for fixed and ``False``
	for not fixed.

#1701 includes an example to showcase the current status.

There are many places at which a change could be introduced to restore the old behavior. The current implementation applies the change at the first possible point, where the list is initially created. Alternatively, a conversion could probably happen in paramset or at the last point in _ModelConfig.suggested_fixed. Please let me know if you would prefer another implementation. I picked this approach for consistency with other modifiers, such that required_parset()["fixed"] consistently returns bool / List[bool].

This also adds tests for the return type behavior for staterror modifiers, I'm happy to remove them or add more for other modifiers if desired.

Since this is about types, I thought about adding type hints for the affected code and am happy to do so if you'd like.

resolves #1701

Checklist Before Requesting Reviewer

• Tests are passing
• "WIP" removed from the title of the pull request
• Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

• Summarize commit messages into a comprehensive review of the PR

* Have the fixed parameters selection in staterror_builder be a list
of Bools over numpy.bool_
* Add type hints for the relevant sections
* Add test to test_modifiers.py to require the fixed parameters
selection list to be Bool

[codecov]

Codecov Report

Merging #1706 (b4f6042) into master (f25b8dd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1706   +/-   ##
=======================================
  Coverage   98.11%   98.11%           
=======================================
  Files          64       64           
  Lines        4246     4249    +3     
  Branches      591      590    -1     
=======================================
+ Hits         4166     4169    +3     
  Misses         46       46           
  Partials       34       34           

Flag	Coverage Δ
contrib	26.33% <87.50%> (+0.05%)	⬆️
doctest	61.04% <87.50%> (+0.02%)	⬆️
unittests	96.16% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/pyhf/modifiers/staterror.py	97.97% <100.00%> (+0.02%)	⬆️
src/pyhf/parameters/paramsets.py	100.00% <100.00%> (ø)
src/pyhf/pdf.py	97.80% <100.00%> (+<0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f25b8dd...b4f6042. Read the comment docs.

[matthewfeickert]

Thanks for this PR @alexander-held. 👍

Since this is about types, I thought about adding type hints for the affected code and am happy to do so if you'd like.

If you'd like to add them we certainly won't complain. :) Though I'm happy to approve this as is.

[alexander-held]

I added type hints for the relevant part of the code, only relating to suggested_fixed. I verified them all at runtime with typeguard as well and they are correct for the examples I tried (#1701 and slight modifications).

[matthewfeickert]

(I'm not sure why the CodeQL check is hanging, but this PR is obviously fine with or without it).