feat: Raise exception if bin-wise modifier data length doesn't match sample data

[matthewfeickert]

Description

Resolves #1216

Raise an InvalidModifier exception if bin-wise modifiers have data that is of a different length than the data for the sample they belong to. The exception attempts to tell the user exactly where the problem is in the spec.

This gets applied to histosys, shapesys, and staterror modifiers if I have this right.

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

* Add a check to bin-wise modifiers to ensure that the modifier data
length is equal to the length of the data in the sample containing
the modifier. If this fails, raise a pyhf.exceptions.InvalidModifier
and try to give the user as much information as possible about what
modifier has failed.
   - Apply this to histosys, shapesys, and staterror
* Add tests to test_modifiers for this modifier behavior.
* Apply isort to all changed files
* Remove unused testing code from test_modifiers.py that was missed
for removal in PR #1625

[matthewfeickert]

I still need to add tests, but @kratsg and @lukasheinrich if you have early feedback on changes they're welcome — I haven't touched the modifiers in a long time.

[codecov]

Codecov Report

Merging #1708 (3aea7ca) into master (d072da0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1708   +/-   ##
=======================================
  Coverage   98.11%   98.12%           
=======================================
  Files          64       64           
  Lines        4249     4267   +18     
  Branches      590      593    +3     
=======================================
+ Hits         4169     4187   +18     
  Misses         46       46           
  Partials       34       34           

Flag	Coverage Δ
contrib	26.27% <24.13%> (-0.07%)	⬇️
doctest	60.83% <37.93%> (-0.22%)	⬇️
unittests	96.17% <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/histosys.py	100.00% <100.00%> (ø)
src/pyhf/modifiers/shapesys.py	100.00% <100.00%> (ø)
src/pyhf/modifiers/staterror.py	98.09% <100.00%> (+0.11%)	⬆️

---

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 d072da0...3aea7ca. Read the comment docs.

[matthewfeickert]

With the spec that @kratsg gave as an example in Issue #1216 as issue_1216.json this PR will give

$ pyhf inspect issue_1216.json 
Traceback (most recent call last):
  File "/home/feickert/.pyenv/versions/pyhf-dev-CPU/bin/pyhf", line 33, in <module>
    sys.exit(load_entry_point('pyhf', 'console_scripts', 'pyhf')())
  File "/home/feickert/.pyenv/versions/3.9.6/envs/pyhf-dev-CPU/lib/python3.9/site-packages/click/core.py", line 1128, in __call__
    return self.main(*args, **kwargs)
  File "/home/feickert/.pyenv/versions/3.9.6/envs/pyhf-dev-CPU/lib/python3.9/site-packages/click/core.py", line 1053, in main
    rv = self.invoke(ctx)
  File "/home/feickert/.pyenv/versions/3.9.6/envs/pyhf-dev-CPU/lib/python3.9/site-packages/click/core.py", line 1659, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/feickert/.pyenv/versions/3.9.6/envs/pyhf-dev-CPU/lib/python3.9/site-packages/click/core.py", line 1395, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/feickert/.pyenv/versions/3.9.6/envs/pyhf-dev-CPU/lib/python3.9/site-packages/click/core.py", line 754, in invoke
    return __callback(*args, **kwargs)
  File "/home/feickert/Code/GitHub/pyhf/src/pyhf/cli/spec.py", line 82, in inspect
    model = ws.model()
  File "/home/feickert/Code/GitHub/pyhf/src/pyhf/workspace.py", line 425, in model
    return Model(modelspec, **config_kwargs)
  File "/home/feickert/Code/GitHub/pyhf/src/pyhf/pdf.py", line 667, in __init__
    modifiers, _nominal_rates = _nominal_and_modifiers_from_spec(
  File "/home/feickert/Code/GitHub/pyhf/src/pyhf/pdf.py", line 148, in _nominal_and_modifiers_from_spec
    finalizd_builder_data[k] = modifiers_builders[k].finalize()
  File "/home/feickert/Code/GitHub/pyhf/src/pyhf/modifiers/histosys.py", line 87, in finalize
    raise InvalidModifier(
pyhf.exceptions.InvalidModifier: The 'WtZ' sample histosys modifier 'syst_wrongbins' has data shape inconsistent with the sample.
WtZ has 'data' of length 4 but syst_wrongbins has 'lo_data' of length 3 and 'hi_data' of length 3.

[matthewfeickert]

@kratsg This is ready for review again now with patch files. I need to run at the moment, but it would be nicer if we could also add newlines at the end of the JSON files.