Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

vendor py.path and py.error #10396

Merged
merged 20 commits into from Oct 21, 2022
Merged

vendor py.path and py.error #10396

merged 20 commits into from Oct 21, 2022

Conversation

asottile
Copy link
Member

@asottile asottile commented Oct 19, 2022

there is one tiny behavioural change -- that is sysexec now raises RuntimeError instead of a sepcialized py.process.cmdexec error -- I think this is probably fine, I doubt anyone is actually using py.path.local.sysexec 馃し

otherwise we could vendor that module too -- but seems not worth

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That particular hack hurts in a special way

import _pytest._py.error as error
import _pytest._py.path as path

sys.modules["py.error"] = error
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks i hate it, but it's a fun hack

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh -- it's basically what apipkg does iirc

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but the overreaching hack into the python import system to be backward compatible in this "yikes this works" way is quite something

@asottile
Copy link
Member Author

I guess I should finish the linting and typing for _pytest._py.path then huh

@nicoddemus
Copy link
Member

nicoddemus commented Oct 20, 2022

@RonnyPfannschmidt so you are OK with vendoring py.path? @The-Compiler any opinions?

I personally like vendoring it, as we can drop py immediately. 馃憤

In addition, we should also port the tests for py.path.local in this vendoring process (or perhaps you plan to do it later, that's why this is a draft).

@asottile asottile force-pushed the pylib-hax branch 2 times, most recently from 5d9f74b to dc0cb0d Compare October 20, 2022 02:18
@francesco-ballarin
Copy link

Hi,
I want to report a side effect of this change when using mypy. py used to be typed, while apparently the vendored library is not. Downstream developers that define pytest hooks may now see mypy errors like:

nbvalx/pytest_hooks_notebooks.py:32: error: Skipping analyzing "py": module is
installed, but missing library stubs or py.typed marker  [import]
    import py
    ^
nbvalx/pytest_hooks_notebooks.py:32: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
nbvalx/pytest_hooks_notebooks.py:412: error: Argument 2 to "collect_file"
becomes "Any" due to an unfollowed import  [no-any-unimported]
    def collect_file(file_path: pathlib.Path, path: py.path.local, parent:...
    ^

I'll leave it to you to decide if it is worth it to add back that support (downstream developers can easily add a couple of ignores, especially knowing that py.path is being deprecated anyway), but I want to report this here for future reference because at the beginning I couldn't wrap my head around what had happened (latest py release was one year ago, and surely I had ran my CI last week without errors) and it took me a while to figure out what was going on.

Cheers,
Francesco

@The-Compiler
Copy link
Member

@francesco-ballarin I opened #10435 for this. Maybe there's an easy way we can fix this, but in here, this is likely to get lost.

The-Compiler added a commit to The-Compiler/pytest-mypy-plugins that referenced this pull request Oct 26, 2022
With pytest 7.2, the remaining parts of the "py.path" library got vendored, to
get rid of the dependency: pytest-dev/pytest#10396

However, this breaks due to pytest_mypy_plugins importing private API:

    File ".../pytest_mypy_plugins/collect.py", line 13, in <module>
        from py._path.local import LocalPath
    ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Use py.path.local instead (the public name of the same type), which is part of
the shim included in pytest.
The-Compiler added a commit to The-Compiler/pytest-mypy-plugins that referenced this pull request Oct 26, 2022
With pytest 7.2, the remaining parts of the "py.path" library got vendored, to
get rid of the dependency: pytest-dev/pytest#10396

However, this breaks due to pytest_mypy_plugins importing private API:

    File ".../pytest_mypy_plugins/collect.py", line 13, in <module>
        from py._path.local import LocalPath
    ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Use py.path.local instead (the public name of the same type), which is part of
the shim included in pytest.
sobolevn pushed a commit to typeddjango/pytest-mypy-plugins that referenced this pull request Oct 26, 2022
With pytest 7.2, the remaining parts of the "py.path" library got vendored, to
get rid of the dependency: pytest-dev/pytest#10396

However, this breaks due to pytest_mypy_plugins importing private API:

    File ".../pytest_mypy_plugins/collect.py", line 13, in <module>
        from py._path.local import LocalPath
    ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Use py.path.local instead (the public name of the same type), which is part of
the shim included in pytest.
guewen added a commit to guewen/pytest-odoo that referenced this pull request Oct 26, 2022
The py dependency has been removed in pytest:
pytest-dev/pytest#10396
moto-timo added a commit to moto-timo/yocto-dockerfiles that referenced this pull request Oct 29, 2022
Since pytest-dev/pytest#10396 the py library is
no longer installed, but dumb-init needs 'py._path'.

Fixes:

ImportError while importing test module '/tmp/tmp.k8zSDBKT6S/dumb-init-1.2.5/tests/child_processes_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/child_processes_test.py:10: in <module>
    from testing import is_alive
testing/__init__.py:11: in <module>
    from py._path.local import LocalPath
E   ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Issue filed upstream:
Yelp/dumb-init#286

Signed-off-by: Tim Orling <tim.orling@konsulko.com>
moto-timo added a commit to moto-timo/yocto-dockerfiles that referenced this pull request Oct 29, 2022
Append 'py' to dumb-init*/requirements-dev.txt

Since pytest-dev/pytest#10396 the py library is
no longer installed, but dumb-init needs 'py._path'.

Fixes:

ImportError while importing test module '/tmp/tmp.k8zSDBKT6S/dumb-init-1.2.5/tests/child_processes_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/child_processes_test.py:10: in <module>
    from testing import is_alive
testing/__init__.py:11: in <module>
    from py._path.local import LocalPath
E   ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Issue filed upstream:
Yelp/dumb-init#286

Signed-off-by: Tim Orling <tim.orling@konsulko.com>
moto-timo added a commit to moto-timo/yocto-dockerfiles that referenced this pull request Oct 29, 2022
Append 'py' to dumb-init*/requirements-dev.txt

Since pytest-dev/pytest#10396 the py library is
no longer installed, but dumb-init needs 'py._path'.

Fixes:

ImportError while importing test module '/tmp/tmp.k8zSDBKT6S/dumb-init-1.2.5/tests/child_processes_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/child_processes_test.py:10: in <module>
    from testing import is_alive
testing/__init__.py:11: in <module>
    from py._path.local import LocalPath
E   ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Issue filed upstream:
Yelp/dumb-init#286

Signed-off-by: Tim Orling <tim.orling@konsulko.com>
moto-timo added a commit to crops/yocto-dockerfiles that referenced this pull request Oct 29, 2022
Append 'py' to dumb-init*/requirements-dev.txt

Since pytest-dev/pytest#10396 the py library is
no longer installed, but dumb-init needs 'py._path'.

Fixes:

ImportError while importing test module '/tmp/tmp.k8zSDBKT6S/dumb-init-1.2.5/tests/child_processes_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/child_processes_test.py:10: in <module>
    from testing import is_alive
testing/__init__.py:11: in <module>
    from py._path.local import LocalPath
E   ModuleNotFoundError: No module named 'py._path'; 'py' is not a package

Issue filed upstream:
Yelp/dumb-init#286

Signed-off-by: Tim Orling <tim.orling@konsulko.com>
simahawk pushed a commit to guewen/pytest-odoo that referenced this pull request Oct 31, 2022
The py dependency has been removed in pytest:
pytest-dev/pytest#10396
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Nov 2, 2022
pytest-dev/pytest#10396

git-svn-id: file:///srv/repos/svn-community/svn@1340954 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Nov 2, 2022
pytest-dev/pytest#10396


git-svn-id: file:///srv/repos/svn-community/svn@1340954 9fca08f4-af9d-4005-b8df-a31f2cc04f65
@d-chambers d-chambers mentioned this pull request Nov 2, 2022
1 task
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Nov 5, 2022
python-py was brought in by older pytest [1][2]

[1] 6687aae
[2] pytest-dev/pytest#10396

git-svn-id: file:///srv/repos/svn-community/svn@1342284 9fca08f4-af9d-4005-b8df-a31f2cc04f65
archlinux-github pushed a commit to archlinux/svntogit-community that referenced this pull request Nov 5, 2022
python-py was brought in by older pytest [1][2]

[1] 6687aae
[2] pytest-dev/pytest#10396



git-svn-id: file:///srv/repos/svn-community/svn@1342284 9fca08f4-af9d-4005-b8df-a31f2cc04f65
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants