Codecov Report

Merging #1467 (e865224) into master (be5ae65) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1467   +/-   ##
=======================================
  Coverage   98.04%   98.04%           
=======================================
  Files          63       63           
  Lines        4145     4145           
  Branches      572      572           
=======================================
  Hits         4064     4064           
  Misses         48       48           
  Partials       33       33           

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 be5ae65...e865224. Read the comment docs.

Relevant RTD build page: https://pyhf--1467.org.readthedocs.build/en/1467/#hello-world

Changes look fine, I only disagree with this line in the changelog:

Use 'pytest' over 'python -m pytest' to avoid adding the current directory to sys.path. Having
the cwd in sys.path invalidates some of the benefits of using a src directory structure for
testing.

You can't invalidate the benefits of a src directory with this; this is the benefit of a src directory. There's no way to import pyhf from src/pyhf by putting the current directory into the path. You would have to add "src" to the PYTHONPATH / sys.path.

Also, if you do coverage testing, I think you'll need an editable install (which is fine, you should be able to do either one).

You can't invalidate the benefits of a src directory with this; this is the benefit of a src directory. There's no way to import pyhf from src/pyhf by putting the current directory into the path. You would have to add "src" to the PYTHONPATH / sys.path.

Yeah, you're right. Edited. Thanks for the catch.

Also, if you do coverage testing, I think you'll need an editable install (which is fine, you should be able to do either one).

Can you elaborate a bit on this? The below seems to be working for generating a coverage file and reporting

    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip setuptools wheel
        python -m pip install --upgrade .[test]

...

    - name: Test with pytest
      run: |
        pytest -r sx --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py

    - name: Report core project coverage with Codecov
      if: github.event_name != 'schedule' && matrix.python-version == 3.9 && matrix.os == 'ubuntu-latest'
      uses: codecov/codecov-action@v2
      with:
        files: ./coverage.xml
        flags: unittests

Why do we need an editable install? (I could be missing something very obvious)

I thought that coverage was picky about the paths that it reports, that is, if it's reporting src/pyhf/... it's happy and not if it's <random long location>/pyhf/.... Could be wrong though, I get CodeCov and coveralls muddled.

CodeCov seems to be happy. :)

I thought that coverage was picky about the paths that it reports, that is, if it's reporting src/pyhf/... it's happy and not if it's <random long location>/pyhf/.... Could be wrong though, I get CodeCov and coveralls muddled.

CodeCov seems to be happy. :)

Ah okay, thanks for the clarification. Agreed, as CodeCov seems happy then I think we're good here. Thanks for taking time to look this over though @henryiii — always appreciate your time and input!