Do not import duplicated modules with --importmode=importlib by nicoddemus · Pull Request #12074 · pytest-dev/pytest

nicoddemus · 2024-03-04T15:09:00Z

Regression brought up by #11475.

Regression brought up by pytest-dev#11475.

nicoddemus · 2024-03-04T15:11:14Z

@flying-sheep I could not reproduce the issue using your reproducer, in 7.4.4, 8.1.0 and this branch I get the same error:

======================================================= ERRORS ========================================================
________________________________ ERROR collecting scanpy/_utils/compute/is_constant.py ________________________________
ImportError while importing test module 'c:\Users\bruno\projects\scanpy\scanpy\_utils\compute\is_constant.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
scanpy\_utils\compute\is_constant.py:12: in <module>
    from ..._compat import DaskArray
.env\lib\site-packages\scanpy\__init__.py:19: in <module>
    from ._utils import check_versions
.env\lib\site-packages\scanpy\_utils\__init__.py:33: in <module>
    from .compute.is_constant import is_constant  # noqa: F401
E   ImportError: cannot import name 'is_constant' from 'scanpy._utils.compute.is_constant' (c:\Users\bruno\projects\scanpy\scanpy\_utils\compute\is_constant.py)

Perhaps this is due to me running Windows? Can you try this branch yourself perhaps?

bluetech

Regardless of whether it fixes @flying-sheep's issue, the change LGTM

flying-sheep · 2024-03-04T15:30:21Z

Ah, sorry, the reproducer didn’t do what I thought it did.

This one works. I’ll check if this PR fixes things:

git clone https://github.com/scverse/anndata.git -b src2
cd anndata
python -m venv .venv
.venv/bin/pip install .[test] pytest@https://github.com/pytest-dev/pytest/releases/download/8.1.0/pytest-8.1.0-py3-none-any.whl
.venv/bin/pytest anndata src/anndata/tests/test_awkward.py::test_view src/anndata/tests/test_backed_sparse.py::test_empty_backed_indexing

FAILED src/anndata/tests/test_awkward.py::test_view[obsm] - Failed: DID NOT WARN. No warnings of type (<class 'anndata._warnings.ImplicitModificationWarning'>,) were emitted.
FAILED src/anndata/tests/test_awkward.py::test_view[varm] - Failed: DID NOT WARN. No warnings of type (<class 'anndata._warnings.ImplicitModificationWarning'>,) were emitted.
ERROR src/anndata/tests/test_backed_sparse.py::test_empty_backed_indexing[zarr-empty_list] - ImportError: cannot import name 'write_zarr' from 'anndata._io.zarr' (/home/phil/Dev/Python/_reproducers/anndata/.venv/lib/python3.11/site-packages/zarr/__init__.py)
ERROR src/anndata/tests/test_backed_sparse.py::test_empty_backed_indexing[zarr-empty_bool_mask] - ImportError: cannot import name 'write_zarr' from 'anndata._io.zarr' (/home/phil/Dev/Python/_reproducers/anndata/.venv/lib/python3.11/site-packages/zarr/__init__.py)

flying-sheep · 2024-03-04T15:36:34Z

OK, I tried

git clone https://github.com/scverse/anndata.git -b src2
cd anndata
python -m venv .venv
.venv/bin/pip install .[test] 'pytest@git+https://github.com/nicoddemus/pytest.git@11475-duplicated-imports'
.venv/bin/pytest anndata src/anndata/tests/test_awkward.py::test_view src/anndata/tests/test_backed_sparse.py::test_empty_backed_indexing

and it worked!

/edit: running all tests via .venv/bin/pytest also worked, so this PR probably covers all cases.

🥳

nicoddemus · 2024-03-04T15:44:50Z

Awesome, thanks @flying-sheep!

aaraney · 2024-03-11T20:52:35Z

I think this may have introduced a different regression. Consider the following project structure with two packages under the ns namespace:

ns
└── python
    ├── bar
    │   ├── ns
    │   │   ├── bar
    │   │   └── test
    │   │       ├── __init__.py
    │   │       ├── bar.py
    │   │       └── test_bar.py
    │   └── pyproject.toml
    └── foo
        ├── ns
        │   ├── foo
        │   └── test
        │       ├── __init__.py
        │       ├── foo.py
        │       └── test_foo.py
        └── pyproject.toml

In pytest==8.0.2, python -m pytest --import-mode=importlib correctly runs the tests from the top level ns directory. In pytest==8.1.1, python -m pytest --import-mode=importlib -o "consider_namespace_packaes=true", results in the following error:

========================== test session starts ===========================
platform darwin -- Python 3.9.16, pytest-8.1.1, pluggy-1.4.0
rootdir: /home/user/pytest-12074
collected 0 items / 2 errors

================================= ERRORS =================================
____________ ERROR collecting python/bar/ns/test/test_bar.py _____________
ImportError while importing test module '/home/user/pytest-12074/python/bar/ns/test/test_bar.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
python/bar/ns/test/test_bar.py:1: in <module>
    from .bar import value
E   ModuleNotFoundError: No module named 'test.bar'
____________ ERROR collecting python/foo/ns/test/test_foo.py _____________
ImportError while importing test module '/home/user/pytest-12074/python/foo/ns/test/test_foo.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
python/foo/ns/test/test_foo.py:1: in <module>
    from .foo import value
E   ModuleNotFoundError: No module named 'test.foo'
======================== short test summary info =========================
ERROR python/bar/ns/test/test_bar.py
ERROR python/foo/ns/test/test_foo.py
!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!
=========================== 2 errors in 0.04s ============================

aaraney · 2024-03-12T13:48:14Z

Just a little bit of a follow up, because I realize that my original comment wasn't the most clear. Below are the contents of test_foo.py and foo.py. test_bar.py and bar.py look nearly identical.

# python/foo/ns/test/test_foo.py
from .foo import value

def test_foo():
    assert value == "foo"

# python/foo/ns/test/foo.py
value = "foo"

nicoddemus · 2024-03-12T14:14:46Z

@aaraney thanks for the report, but could you open a new issue please (you can refer to this one there)?

I will take a look at it ASAP. 👍

aaraney · 2024-03-12T14:15:49Z

@nicoddemus, for sure! Thanks for taking a look at this!

…dev#12074) Regression brought up by pytest-dev#11475.

tacaswell · 2024-07-03T20:58:38Z

This appears to have broken assertion re-writing with editable installs.

tacaswell · 2024-07-03T21:03:25Z

This is fixed by #12313

RonnyPfannschmidt · 2024-07-04T07:45:12Z

src/_pytest/pathlib.py

            pass
        else:
+            # If the given module name is already in sys.modules, do not import it again.
+            with contextlib.suppress(KeyError):


just taking a look again, i believe this should trigger a import mismatch if the file of the loaded module and the current path are different

Do not import duplicated modules with --importmode=importlib

f408b94

Regression brought up by pytest-dev#11475.

bluetech approved these changes Mar 4, 2024

View reviewed changes

nicoddemus added the needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch label Mar 4, 2024

flying-sheep mentioned this pull request Mar 4, 2024

Moving source directory to src/anndata scverse/anndata#1151

Merged

nicoddemus merged commit 03e5471 into pytest-dev:main Mar 4, 2024

nicoddemus added backport 8.1.x and removed needs backport applied to PRs, indicates that it should be ported to the current bug-fix branch labels Mar 4, 2024

nicoddemus deleted the 11475-duplicated-imports branch March 4, 2024 15:45

github-actions bot mentioned this pull request Mar 4, 2024

[8.1.x] Do not import duplicated modules with --importmode=importlib #12077

Merged

ksunden mentioned this pull request Mar 4, 2024

pin pytest matplotlib/matplotlib#27858

Merged

5 tasks

aaraney mentioned this pull request Mar 12, 2024

chore: pin pytest~=8.0.0 until pytest/12074 is fixed NOAA-OWP/DMOD#542

Merged

aaraney mentioned this pull request Mar 12, 2024

pytest==8.1.1 Import regression in some namespace package layouts #12112

Closed

flying-sheep pushed a commit to flying-sheep/pytest that referenced this pull request Apr 9, 2024

Do not import duplicated modules with --importmode=importlib (pytest-…

ab951af

…dev#12074) Regression brought up by pytest-dev#11475.

RonnyPfannschmidt reviewed Jul 4, 2024

View reviewed changes

Uh oh!

Comments

Conversation

nicoddemus commented Mar 4, 2024

Uh oh!

nicoddemus commented Mar 4, 2024

Uh oh!

bluetech left a comment

Choose a reason for hiding this comment

Uh oh!

flying-sheep commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flying-sheep commented Mar 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicoddemus commented Mar 4, 2024

Uh oh!

aaraney commented Mar 11, 2024

Uh oh!

aaraney commented Mar 12, 2024

Uh oh!

nicoddemus commented Mar 12, 2024

Uh oh!

aaraney commented Mar 12, 2024

Uh oh!

tacaswell commented Jul 3, 2024

Uh oh!

tacaswell commented Jul 3, 2024

Uh oh!

RonnyPfannschmidt Jul 4, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

flying-sheep commented Mar 4, 2024 •

edited

Loading

flying-sheep commented Mar 4, 2024 •

edited

Loading