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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix installing PEP 561 stub-only packages #2466

Merged
merged 1 commit into from Dec 8, 2023

Conversation

lukaszmoroz
Copy link
Contributor

@lukaszmoroz lukaszmoroz commented Dec 6, 2023

Pull Request Checklist

  • A news fragment is added in news/ describing what is new.
  • Test cases added for changed code.

Describe what you have changed in this PR.

I have the following, very specific setup:

foo-bar:
foo/
    bar/
        __init__.py

foo-baz:
foo/
    baz.cpython-39-x86_64-linux-gnu.so
    baz/
        __init__.pyi

The foo-bar dependency provides a normal python module foo.bar inside a namespace package foo.
The foo-baz dependency provides an extension module foo.baz along with type stubs (.pyi), also inside namespace foo.

PDM currently mishandles those when installing with symlink support:

PDMWarning: Overwriting existing package: [...]

That's because the logic for installing namespace packages doesn't mark foo/ in the foo-baz dependency as a namespace package since it not being a package at all:

elif not _is_namespace_package(root) and not link_individual:
# A package, create link for the parent dir and don't proceed
# for child directories
if os.path.exists(destination_root):
warnings.warn(f"Overwriting existing package: {destination_root}", PDMWarning, stacklevel=2)

def _is_namespace_package(root: str) -> bool:
if not _is_python_package(root):
return False

def _is_python_package(root: str | Path) -> bool:
for child in Path(root).iterdir():
if (
child.is_file()
and child.suffix in (".py", ".pyc", ".pyo", ".pyd")

I see 2 issues here with PDM not being able to detect:

  • packages providing only extension modules (.so)
  • packages providing only type stubs (.pyi) (PEP 561)

This fix hopefully addresses the second issue, which is enough for my use case.

I don't know whether extension modules can be safely detected similar way by the presence of .so files (not all .so files necessarily indicate a python module), so I didn't touch that one.

@frostming
Copy link
Collaborator

Can you please add a news fragment under news/?

Copy link

codecov bot commented Dec 7, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (82a4574) 84.55% compared to head (d8ccc40) 84.55%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2466   +/-   ##
=======================================
  Coverage   84.55%   84.55%           
=======================================
  Files         104      104           
  Lines       10280    10280           
  Branches     2247     2247           
=======================================
  Hits         8692     8692           
  Misses       1114     1114           
  Partials      474      474           
Flag Coverage Δ
unittests 84.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lukaszmoroz
Copy link
Contributor Author

I came across this now:
#822 (comment)

Given how many extension modules are out there in scientific computing, it's likely that it will bite someone again.

@frostming frostming merged commit 47e8c69 into pdm-project:main Dec 8, 2023
1 check passed
@j178 j178 mentioned this pull request Apr 3, 2024
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

2 participants