scikit-hep / pyhf

pure-Python HistFactory implementation with tensors and autodiff
https://pyhf.readthedocs.io/
Apache License 2.0
279 stars 83 forks source link

Add coverage to pdf.Model.expected_auxdata and pdf.Model.mainlogpdf #603

Open matthewfeickert opened 4 years ago

matthewfeickert commented 4 years ago

Description

In PR #596 the codecov/patch test failed as the relative coverage was below the total project coverage. This was confusing, as none of the code was new and just some lines were getting switched out. However, as "relative" coverage in Codecov is defined as

Coverage concerning only lines adjusted in the commit diff (aka the diff coverage)

Read as "The lines I changed in this commit are 72% covered."

then this means that if the lines that were changed in the commit were not already covered by a test then they will be counted against the PR's relative coverage.

The lines in question for causing the relative coverage to fail were

https://github.com/diana-hep/pyhf/blob/33282e1dd5b260088fc8f9b12ee6da8dadf87682/src/pyhf/pdf.py#L569-L570

and

https://github.com/diana-hep/pyhf/blob/33282e1dd5b260088fc8f9b12ee6da8dadf87682/src/pyhf/pdf.py#L592-L593

If one runs

git grep "expected_auxdata" tests/
git grep "mainlogpdf" tests/

no tests are found that contain them. I think this was the cause of the failing tests, and can be fixed if tests are added for these methods.

matthewfeickert commented 4 years ago

cc @kratsg