pytest fails for source installs that don't use `pip install --editable .`

[matthewfeickert]

Strangely it seems that if we install from source with

python -m pip install .[test]

pytest will fail with things like

==================================== ERRORS ====================================
_____________________ ERROR collecting src/pyhf/compat.py ______________________
/opt/hostedtoolcache/Python/3.8.10/x64/lib/python3.8/site-packages/_pytest/runner.py:311: in from_call
    result: Optional[TResult] = func()
/opt/hostedtoolcache/Python/3.8.10/x64/lib/python3.8/site-packages/_pytest/runner.py:341: in <lambda>
    call = CallInfo.from_call(lambda: list(collector.collect()), "collect")
/opt/hostedtoolcache/Python/3.8.10/x64/lib/python3.8/site-packages/_pytest/doctest.py:532: in collect
    module = import_path(self.fspath)
/opt/hostedtoolcache/Python/3.8.10/x64/lib/python3.8/site-packages/_pytest/pathlib.py:544: in import_path
    raise ImportPathMismatchError(module_name, module_file, path)
E   _pytest.pathlib.ImportPathMismatchError: ('pyhf.compat', '/opt/hostedtoolcache/Python/3.8.10/x64/lib/python3.8/site-packages/pyhf/compat.py', PosixPath('/home/runner/work/pyhf/pyhf/src/pyhf/compat.py'))

and that it will only pass if we install from source in editable mode

python -m pip install --editable .[test]

Originally posted by @matthewfeickert in https://github.com/scikit-hep/pyhf/issues/1453#issuecomment-841541604

It would be good to understand why this is, and if possible change it. The tests needing things to be installed with pip install --editable . seems strange and bad.

[kratsg]

You might want to check if you have __pycache__ files first, and remove those, before running pytest?

[matthewfeickert]

You might want to check if you have __pycache__ files first, and remove those, before running pytest?

So I don't think that is it, because this was happening in CI. The error above is from the CI logs, so there shouldn't be any __pycache__ just from running python -m pip install .[test], right?

[matthewfeickert]

What's extra strange is if I do

$ python -m pip install .[test]
$ python -m pytest -r sx tests/test_optim.py

everything passes. It is only when I try to run everything

$ python -m pytest -r sx --ignore tests/benchmarks/ --ignore tests/contrib --ignore tests/test_notebooks.py

that things start to fail. :?

[matthewfeickert]

I'm not sure (need to test) but I think it is because we have src in our testpaths in pyproject.toml

https://github.com/scikit-hep/pyhf/blob/546022254a526ea62af8cbc6594ec881cddfa9f1/pyproject.toml#L41-L44

[henryiii]

I would expect that's it. You should rely on Python packaging to handle paths, not manipulate them yourself. I assume "src" is there because you have doctests, but I think those are supposed to run through the installed version (haven't used them much, since it's hard to write complete enough examples in doctests to run them in tests).

[matthewfeickert]

Twitter thread for future reference: https://twitter.com/HEPfeickert/status/1394466047202365444

[matthewfeickert]

I assume "src" is there because you have doctests, but I think those are supposed to run through the installed version

Yeah, exactly this: https://github.com/scikit-hep/pyhf/pull/1467#discussion_r634002984

We've made pretty good use of them as additionally covering the docstrings to make sure that if something changes we don't have a docstring example get into the docs that gets out of date / is wrong. I'm not sure how to reconcile these two things at the moment, so I'll probably have to look at this more after vCEHP. :/

[diazona]

I spent an hour or so digging into this and I found some useful info. pytest pulls in tests/conftest.py as part of its first pass, before it loads any other test files or source code, and in particular, before it adds src/ to sys.path for doctest discovery. Since conftest.py imports pyhf, it will load the versions of the modules from the installed version of the package, i.e. lib/pythonX.Y/site-packages/pyhf/. Then later, when pytest goes looking for (doc)tests under src/, it prepends src/ to sys.path (NB for doctests, it always prepends, ignoring the --import-mode CLI option) in an effort to import the .py files from within src/, but those modules already exist in sys.modules, having been imported from the installed version. The conflict between the already-imported module and the file that pytest is trying to newly import seems to cause the mismatch error.

So ultimately, if you can remove the dependency of tests/conftest.py on the pyhf code, that should do the trick. I did a quick "feasibility study" by removing the whole parameter list of backend() and the pyhf import, and it seems to allow things to proceed past test collection, which is a good sign, though I did get a test to fail so maybe there are other things to fix.

[matthewfeickert]

So this old answer from 2017 https://github.com/pytest-dev/pytest/issues/2287#issuecomment-284200894

The standard procedure is to use a virtual environment (which travis already sets up for you automatically) and install your project in development mode: ... Then both python and pytest will be able to find pets no matter where you execute them from.

seems to imply that using an editable install is expected. I will admit that I still don't fully understand why this is required, but I'll reread this again.

[matthewfeickert]

Apparently if you have a library with a src structure then pytest won't work with it if you just run pytest from the top level without also a directionality flag (e.g. pytest tests ).

https://stackoverflow.com/a/50156706/8931942

[henryiii]

This is one of the main reasons to have a src structure in the first place. It is only valid to use an "installed" package. Otherwise you give up all the packaging can offer, like dependencies, versioning, data files, extensions, and custom file manipulations. If you don't have the src, then it can be really hard to get PyTest to even use the installed version, it loves local paths.

If you really know what you are doing, PYTHONPATH=src pytest. But please use vents and editable installs. Or nox.

[henryiii]

Ahh, I see you are dealing with tests inside src? Normally I'd say that's a bad idea, since they have to get installed with your package, but it's Docstring tests?

[matthewfeickert]

Ahh, I see you are dealing with tests inside src? Normally I'd say that's a bad idea, since they have to get installed with your package, but it's Docstring tests?

Yeah, I agree this was part of the motivation to move to a src setup back in the day. We have been running pytest with doctest to test the docstrings

https://github.com/scikit-hep/pyhf/blob/44595108cb66827cf8921bc323b0760b794fae23/pyproject.toml#L44

but maybe I can find a way to run doctest with pytest separately and then run the normal tests in a way that doesn't require an editable install. If I can do this by manually specifying things then I can remove src from testpaths

https://github.com/scikit-hep/pyhf/blob/44595108cb66827cf8921bc323b0760b794fae23/pyproject.toml#L45-L48

which would be perferable.