ModuleNotFoundError when using --doctest-modules, a src/ layout, and --import-mode=importlib (and no editable mode install)
#11475
Comments
|
I just wanted to add that we are hitting this issue as well, but we are using editable installs, so it looks like this issue affects both the case with editable and non-editable installs of the project source files. Just to be clear, our project is:
We more or less exactly follow the suggested layout and settings in https://docs.pytest.org/en/7.1.x/explanation/goodpractices.html#tests-outside-application-code, and everything was working fine until attempting to run doctests within the application code using |
|
Yeah, could this please be added to the 8.0 milestone? This might require a new setting or some breaking change, so this is the ideal moment |
|
Does it work better if you use |
|
[tool.pytest.ini_options]
addopts = [
"--strict-markers",
"--doctest-modules",
"--pyargs",
]
testpaths = [
"anndata", # doctests in docstrings (module name due to --pyargs)
"./src/anndata/tests", # unit tests
"./docs/concatenation.rst", # further doctests
]
|
|
In the issue subject you write If you give me a bit of instructions how to reproduce this locally, I can take a look. |
|
Sorry, I muddled my project (scverse/anndata#1151) too much with this issue. I’m trying to be clear now. Adding What needs to work for this issue to be considered fixed: [build-system]
build-backend = "hatchling.build"
requires = ["hatchling", "hatch-vcs"]
[project]
name = "my_module"
version = "0.1.0"
[tool.pytest.ini_options]
addopts = [
"--import-mode=importlib",
"--strict-markers",
"--doctest-modules",
"--pyargs",
]
testpaths = [
"my_module", # or without `--pyargs`: "./src/my_module"
"./tests",
]What would be nice if it worked for migrating our project:...
[tool.pytest.ini_options]
addopts = [
"--import-mode=importlib",
"--strict-markers",
"--doctest-modules",
"--pyargs",
]
testpaths = [
"my_module",
"./src/my_module/tests",
] |
|
Very weird, I just can’t find a good reproducer. In our project, it fails: https://dev.azure.com/scverse/anndata/_build/results?buildId=5213&view=logs&jobId=5ea502cf-d418-510c-3b5f-c4ba606ae534&j=5ea502cf-d418-510c-3b5f-c4ba606ae534&t=5091afe0-1787-5ef7-c6a1-ce6dc06b30a7 but using the “reproducer” I so confidently outlined above, everything works. |
|
OK, I minified the repo somewhat. I assume the problem is that even with https://github.com/flying-sheep/pytest-doctest-import-mismatch I see errors like this: ____ ERROR collecting _core/merge.py ____
/home/phil/.local/share/hatch/env/virtual/anndata/kX3YdB0h/anndata/lib/python3.11/site-packages/anndata/_core/merge.py:28: in <module>
from ..compat import _map_cat_to_str
E ModuleNotFoundError: No module named 'home.phil..local.share.hatch.env.virtual.anndata.kX3YdB0h.anndata.lib.python3.11.site-packages.anndata.compat'; 'home.phil..local.share.hatch.env.virtual.anndata.kX3YdB0h.anndata.lib.python3.11.site-packages.anndata' is not a package
____ ERROR collecting _core/raw.py ____
/home/phil/.local/share/hatch/env/virtual/anndata/kX3YdB0h/anndata/lib/python3.11/site-packages/anndata/_core/raw.py:9: in <module>
from .aligned_mapping import AxisArrays
E ImportError: cannot import name 'AxisArrays' from 'home.phil..local.share.hatch.env.virtual.anndata.kX3YdB0h.anndata.lib.python3.11.site-packages.anndata._core.aligned_mapping' (/home/phil/.local/share/hatch/env/virtual/anndata/kX3YdB0h/anndata/lib/python3.11/site-packages/anndata/_core/aligned_mapping.py)The reproducer is by no means minimal yet, and I can make it smaller for sure, but maybe it’s helpful before i get to that already. |
|
(First, consider editing the original post to point to the reproduction; the Thanks for creating a reproduction. I tried it now. I was able to reproduce with pytest 7.4, but with pytest main it works (only if using venv name However, there's an issue I noticed: If the path contains dots, e.g. Dot problem
Python module names (individual components) cannot contain dots, and the I tried "escaping" the dots by replacing them with Exec module problem
This is possibly another problem with @nicoddemus Do you have any idea about this? |
flying-sheep
changed the title
pip install without -e, --doctest-modules, a src/ layout, and --import-mode=importlibpip install without -e, --doctest-modules, a src/ layout, and --import-mode=importlib
fixed
Well, unless I’m missing something, the same package is still imported twice with this approach, right? Which might cause problems similar to #11306 when the two versions try to access some shared resource. (e.g. let’s say they both open a database connection or write a file on import time – not super good style, but possible) |
|
@nicoddemus any idea? |
flying-sheep
changed the title
pip install without -e, --doctest-modules, a src/ layout, and --import-mode=importlibpip install with --doctest-modules, a src/ layout, --import-mode=importlib, and without -e
flying-sheep
changed the title
pip install with --doctest-modules, a src/ layout, --import-mode=importlib, and without -e--doctest-modules, a src/ layout, and --import-mode=importlib (and no editable mode install)
|
I think no “escaping” hackery is necessary. We run So how about we just infer the correct module name of the parent module using |
|
Sorry for the long post, spent the last 3 hours working on this and I'm afraid there's no simple "fix" for this case. The short answer is that pytest uses The introduction of
(Full explanation in the docs). On a non-src layout: One of the reasons why changing On This is not a problem: adding The reproducer repository is using @flying-sheep, by changing your configuration slightly I was able to make your reproduce run without problems in pytest
Here's the diff: diff --git a/pyproject.toml b/pyproject.toml
index 81389c5..d15f40a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -21,12 +21,12 @@ dependencies = [
[tool.hatch.build]
exclude = [
+ 'src/anndata/tests/conftest.py',
'src/anndata/tests/test_*.py',
]
[tool.pytest.ini_options]
addopts = [
- '--import-mode=importlib',
'--doctest-modules',
'--pyargs',
]Follows is a long description of the overall problem. The introduction of
(Full explanation in the docs). One important thing in that description is *test modules and conftests. Before we introduced the The To import that file, we:
This works well when used to import just test modules and conftests, but will create divergences when used import application code. In the reproducer repo, we are trying to import Probably one solution is to revisit all places where we manually import modules, and possibly use "importlib" (when configured) for tests+conftests or fallback to the other mechanisms for normal modules, but this feels extremely risky given the millions of test suites out there. The
But here we are. I'm not sure how to proceed here. Perhaps we can right away add a warning on I will spend some more time trying to see if there's a simple and safe solution to this general mess. |
|
Maybe I missed it when reading your summary, but isn’t my PR a simple and safe solution? It gets rid of this iffy strategy:
whenever some module does live below one of the |
|
I will double check, but @bluetech's concern is valid: search through It does not invalidate it completely of course, but it is something we should consider. As I said, I will double check it later. 👍 |
|
I don‘t think it invalidates it in the slightest before someone has measured the impact 😉 Also keep in mind that I outlined some strategies to make it faster. I don‘t think we should even consider sacrificing correctness before having tried out all other sensible avenues. |
Oh I agree 100%, I didn't mean to imply that we were not applying your fix based solely on any performance problems that we have a gut feeling about -- I am just thinking there might be some other solution. One such solution that occurred to me, and I plan to test, is that when using importlib, we first try to import the module normally, without altering |
|
I plan to spend more time this week on this, and solve this once and for all (with your fix or something else). |
I’m confused, I thought importlib’s point is that it doesn’t alter sys.path?
Another possibility is to just be able to make PyTest aware of the module root(s) the project has, e.g. |
Yep, what I mean is just try to import the module, without changing
That would be another solution too. |
|
Proposing a definite solution (IMHO) in #11997. 👍 |
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This also makes `importlib` respect namespace packages. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This also makes `importlib` respect namespace packages. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This also makes `importlib` respect namespace packages. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
|
Sadly the changes you made to the PR after I checked with my project broke it again. I should have checked again … Somehow if running with So e.g.
you can see some of the issues here: https://dev.azure.com/scverse/anndata/_build/results?buildId=5905&view=logs&j=5ea502cf-d418-510c-3b5f-c4ba606ae534 But yeah, I’m pretty sure that the root cause is that somehow things end up being re-imported that should be in the module cache. Any idea what could be causing that? I’m happy to work on a fix myself, but it would be great to have a pointer. |
|
I haven't looked at your failures but IIRC you are using namespace packages, did you set the new |
|
Argh, really sorry to hear that @flying-sheep. I did test the fix in your reproducible example before merging, but seems there is still something to adjust. Looking at the code I suspect there might be a bug there indeed, seems we should be checking if Lines 542 to 544 in 71849cc Like it is being done here: Lines 550 to 552 in 71849cc I will check this tonight. Sorry again for the breakage. 😕 |
|
I’m super grateful that you’re working on getting this resolved. I’m certain once this bug is fixed, this will make using pytest a blast in all kinds of (actual and not-so-)corner cases. I’ll test the fix with our projects, which should be complex-enough real-world examples to prove that it works in a lot of cases. E.g. one easy reproducer is git clone https://github.com/scverse/scanpy.git
cd scanpy
git checkout 14555ba48537995acaa381b8b6ad5fc41e612510 # current main
python -m venv .venv
.venv/bin/pip install .[test] pytest==8.1.0
.venv/bin/pytest scanpy/_utils/compute/is_constant.py |
Regression brought up by pytest-dev#11475.
This is required as of pytest >= 8.1.0. See pytest-dev/pytest#11475
This is required as of pytest >= 8.1.0. See pytest-dev/pytest#11475
|
Note that the documentation of this feature mentions that the default value of |
|
The documentation is talking about the |
…chanism As detailed in pytest-dev#11475 (comment), currently with `--import-mode=importlib` pytest will try to import every file by using a unique module name, regardless if that module could be imported using the normal import mechanism without touching `sys.path`. This has the consequence that non-test modules available in `sys.path` (via other mechanism, such as being installed into a virtualenv, PYTHONPATH, etc) would end up being imported as standalone modules, instead of imported with their expected module names. To illustrate: ``` .env/ lib/ site-packages/ anndata/ core.py ``` Given `anndata` is installed into the virtual environment, `python -c "import anndata.core"` works, but pytest with `importlib` mode would import that module as a standalone module named `".env.lib.site-packages.anndata.core"`, because importlib module was designed to import test files which are not reachable from `sys.path`, but now it is clear that normal modules should be imported using the standard mechanisms if possible. Now `imporlib` mode will first try to import the module normally, without changing `sys.path`, and if that fails it falls back to importing the module as a standalone module. This also makes `importlib` respect namespace packages. This supersedes pytest-dev#11931. Fix pytest-dev#11475 Close pytest-dev#11931
…dev#12074) Regression brought up by pytest-dev#11475.
reproducer
We’d like to run our tests on an installed package instead of the development version, as we delete some files in our build process. We therefore don’t use editable installs.
I think the problem is that there seems to be no way to configure the import root used with the importlib import mode. The project rootdir gets passed here instead of something user configurable:
pytest/src/_pytest/doctest.py
Lines 545 to 549 in c34eaaa
Which means that the following code will run
module_name = module_name_from_path('<rootdir>/src/anndata/_types.py', '<rootdir>'), i.e.module_name = 'src.anndata._types'when it should be'anndata._types'pytest/src/_pytest/pathlib.py
Lines 524 to 525 in 9f3bdac
Without
src/layout, this works accidentally, as themodule_namehappens to match the real module name, and the module only gets imported once.Pytest 7.4.2
The text was updated successfully, but these errors were encountered: