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

Supporting missing backends in tests #1454

Open matthewfeickert opened 3 years ago

matthewfeickert commented 3 years ago

Description

This is motivated by a discussion with @henryiii about why we don't currently support Python 3.9. My current understanding of the problem is that TensorFlow just started supporting Python 3.9 in v2.5.0 (which was released today (2021-05-13)) and we currently cap TensorFlow to v2.2.x

https://github.com/scikit-hep/pyhf/blob/6b769fd6f5e1473deba2b4c55d49ebdb3db5b447/setup.py#L5-L8

given reasons laid out (not neatly) in Issue #293.

@henryiii points out that capping all the other backends at what TensorFlow supports isn't great as then you're limited by a library that you're not even using. To support this though we need to change the way that we run the tests as I don't think (could be wrong) that it is possible for us to run through the test suite with a backend not installed due to it not supporting a Python runtime.

@lukasheinrich @kratsg Thoughts here on how to proceed?

matthewfeickert commented 3 years ago

To support this though we need to change the way that we run the tests as I don't think (could be wrong) that it is possible for us to run through the test suite with a backend not installed due to it not supporting a Python runtime.

Also, yeah, I just tried this and got the expected error at the install stage

ERROR: Could not find a version that satisfies the requirement tensorflow~=2.2.1; extra == "test" (from pyhf[test]) (from versions: 2.5.0rc0, 2.5.0rc1, 2.5.0rc2, 2.5.0rc3, 2.5.0)
ERROR: No matching distribution found for tensorflow~=2.2.1; extra == "test"
alexander-held commented 1 year ago

An additional benefit of this would be that developers could run a subset of tests using a significantly more lightweight environment (see https://github.com/scikit-hep/pyhf/issues/2111#issuecomment-1431659334).

kratsg commented 11 months ago

To support this though we need to change the way that we run the tests as I don't think (could be wrong) that it is possible for us to run through the test suite with a backend not installed due to it not supporting a Python runtime.

One solution is to pass in flags from the command line for pytest to enable tests allowed for a specific backend to run. This shouldn't be too hard to code up in actuality.

alexander-held commented 11 months ago

pytest supports this in a pretty straightforward way, cabinetry example here: https://github.com/scikit-hep/cabinetry/blob/80b4af15c3c8b9738afe1be5450ac3e9d83a12eb/tests/conftest.py#L419-L438. This adds a --runslow and a decorator to add to tests.

matthewfeickert commented 6 months ago

So it seems that PR https://github.com/scikit-hep/pyhf/pull/2340 only partially fixed this under the case where the backend to be skipped must still be installed. In Issue https://github.com/scikit-hep/pyhf/issues/2372, when I've been running on a test branch and trying to run with the following configuration

    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip setuptools wheel
        # No TensorFlow for Python 3.12 yet
        python -m pip install --upgrade ".[jax,torch,minuit,xmlio,contrib,shellcomplete,test]"

    - name: List installed Python packages
      run: python -m pip list

    - name: Test with pytest and coverage
      run: |
        coverage run --module pytest --disable-backend tensorflow --ignore tests/contrib --ignore tests/benchmarks --ignore tests/test_notebooks.py

this fails immediatley with

...
____________________ ERROR collecting tests/test_tensor.py _____________________
ImportError while importing test module '/Users/runner/work/pyhf/pyhf/tests/test_tensor.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/importlib/__init__.py:90: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_tensor.py:5: in <module>
    import tensorflow as tf
E   ModuleNotFoundError: No module named 'tensorflow'
...

as there is not tensorflow out for Python 3.12 yet. So we need to be able to make this so that an import is not event attempted, or that if it is, the exception is caught and handeled.