FIX: Test against a full install, not an editable install

[connortann]

Overview

Fixes #3020.

• Removes the --editable / -e flag from the pip install command during CI, to trigger a full wheel build & install.
• Changes the pytest invocation from to avoid adding the current working directory to sys.path
• Changes the pytest import mode from prepend to append

This is a rather temporary fix to an important problem, so I would advocate for switching to a src layout as strongly recommended by pytest.

Supports #2979.

[PrimozGodec]

I could reproduce the error locally, removing the development installation and installing with pip install .. For me, the case was when I ran import (python -c "import shap;print(shap.__file__)") from the project dir, it imported the development version, but if I moved somewhere else, it imported site-packages versions, which looks ok (when in package dir there shap dir exists, and it has priority against the installed version).

For test according to this the solution is either:

1. to remove __init__.py from the root of tests dir.
2. move tests to the module and call them from the module with https://docs.pytest.org/en/7.1.x/example/pythoncollection.html#interpreting-cmdline-arguments-as-python-packages

For now, I suggest the first solution. Can you try this in this PR?

[connortann]

I don't think the issue in that stackoverflow thread is related to the issue here, but lets try deleting test/__init__.py anyway to confirm. EDIT: the issue remains. As per this job the wrong library is still imported.

If the shap directory is in the current working directory, and if that directory is on sys.path, then that's always going to take precedence over what is installed in ..../sys-modules.

From the pytest docs:

Generally, but especially if you use the default import mode prepend, it is strongly suggested to use a src layout. Here, your application root package resides in a sub-directory of your root, i.e. src/mypkg/ instead of mypkg.

This layout prevents a lot of common pitfalls and has many benefits, which are better explained in this excellent blog post by Ionel Cristian Mărieș.

[connortann]

The correct library is being picked up now the pytest invocation has changed. I expect that this PR will pass once #3021 is merged.

[codecov]

Codecov Report

Merging #3017 (b3a518b) into master (1259272) will decrease coverage by 54.83%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #3017       +/-   ##
==========================================
- Coverage   54.82%   0.00%   -54.83%     
==========================================
  Files          90      90               
  Lines       12902   12902               
==========================================
- Hits         7074       0     -7074     
- Misses       5828   12902     +7074     

see 75 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dsgibbons]

LGTM

[connortann]

@dsgibbons I think this PR may have inadvertently broken code coverage measurements. I think this should be fixable by "combining paths" in the coverage configuration. Attempting a fix on #3033