@lhenkelm while you're working on this can you add some commentary as well? It isn't really clear why all of what is in here so far is strictly needed.

@matthewfeickert the changes are those needed to get access to the ML estimates of the parameters in a call to pyhf.infer.hypotest. The calls stack up like so:
hypotest calls AsymptoticCalculator.teststatistic calls generate_asimov_data and (e.g.) qmu_tilde:

• So it is necessary that generate_asimov_data and all the test statistics are able to return fitted params. (04b0c3b, a2f1c86)
• These params then need to be handed back up through the calculator, which is what the new AsymptoticCalculator.fitted_pars is for (e804185).
• Then finally hypotest needs the option to return the parameters as well (84a8af8).

So the changes to the implementation code are minimal. The bulk of the new lines are tests and docstrings,
which IMO are necessary because all the changes are in the user-visible behavior of documented parts of the API.

I tried to keep the new tests small/generic enough so that they are robust against new changes while covering all of the new behavior. But you guys have the experience of what parts change a lot and how, so let me know if the tests are overbearing!

For the docstrings I tried to be super explicit about describing how the return values change with new option and give plenty of examples for people that find those more useful. So in that sense I think these are necessary changes, but if you think it makes the docstrings too bloated, please tell me!

regarding the tests: they all pass locally, except one unrelated test in test_notebooks.py that also fails when I run the test suite on master.

thanks @lhenkelm ! on first glance this looks good, I'll do a more detailed review asap

I wonder whether at some point it makes sense to use data classes + strutural pattern matching (PEP 622 ) to replace tuple returns

One thing that we should discuss is whether we can somehow unify the "data classes" with cabinetry

i.e. BestFitParams (or TestStatisticFitResults see my comment above), OptimizerResult,cabinetry's FitResult etc cc @alexander-held

I think it would be great to harmonize where possible. Being able to do a infer.mle.fit fit and plug the result straight into a cabinetry visualization (instead of having to use the cabinetry.fit API, or having to pack up the fit result into the object cabinetry expects) seems very useful. I would be happy to try and find a common solution to the packaging of the return values.

Codecov Report

Merging #1554 (7f69d53) into master (9358f85) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1554      +/-   ##
==========================================
+ Coverage   97.70%   97.71%   +0.01%     
==========================================
  Files          63       63              
  Lines        4048     4067      +19     
  Branches      576      581       +5     
==========================================
+ Hits         3955     3974      +19     
  Misses         54       54              
  Partials       39       39              

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

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 9358f85...7f69d53. Read the comment docs.

I appreciate the 100% coverage here (thanks a lot!). We'll merge this in once we get the all clear.

Thanks for the PR @lhenkelm!