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

PEP420 namespaces aren't recognized #740

Open
charlesnicholson opened this issue Jun 24, 2024 · 10 comments · Fixed by #753 or #798
Open

PEP420 namespaces aren't recognized #740

charlesnicholson opened this issue Jun 24, 2024 · 10 comments · Fixed by #753 or #798
Assignees
Labels
bug Something isn't working

Comments

@charlesnicholson
Copy link

In our Python ecosystem, we use PEP420 namespace packages to prefix all of our packages with companyname to avoid package name collisions with third-party packages. For example, our directory structures might look like:

src/
  company.foo/
    company/
      foo/
        foo.py
    pyproject.toml

The pyproject.toml file is pretty vanilla:

[project]
name = "company.foo"
version = "0.0.1"
dependencies = [ ... ]

[build-system]
requires = ["setuptools", "wheel"]
build-backend = "setuptools.build_meta"

When running deptry . from the terminal with the current working directory set to src/company.foo, I get this error:

Assuming the corresponding module name of package 'company' is 'company'. Install the package or configure a package_module_name_map entry to override this behaviour.

This warning makes it seem like deptry can't understand these kinds of packages yet, since the package names aren't ambiguous and are, as best I can tell, conforming to https://peps.python.org/pep-0420/#nested-namespace-packages.

OS: macOS Sonoma 14.5
Language Version: Python 3.11
Poetry version: we don't use poetry. (We use python and uv, but uv plays no role in our package source layouts).

Thanks for deptry! Hopefully it will help us stop shipping broken wheel sets... :-|

@charlesnicholson charlesnicholson added the bug Something isn't working label Jun 24, 2024
@mkniewallner mkniewallner self-assigned this Jul 5, 2024
@mkniewallner
Copy link
Collaborator

This should be fixed in 0.16.2, if you want to check.

@charlesnicholson
Copy link
Author

Thanks for looking into it! It still seems to be happening, unfortunately. I threw together a minimal reproduction in a public repo that you can clone and investigate, if you have the appetite for it:

https://github.com/charlesnicholson/deptry-pep420-repro

Also note the commented-out line in the repro.sh, is it possible that the ROOT argument isn't working as expected? Apologies if I'm misunderstanding the intended usage.

@mkniewallner
Copy link
Collaborator

Thanks for taking the time to create a reproduction repository, really appreciate it!

I won't be able to take a look right away, but will be during the weekend. In the meantime, I'll re-open the issue as it is not yet fixed.

@mkniewallner mkniewallner reopened this Jul 5, 2024
@charlesnicholson
Copy link
Author

Happy to! I set up a manual CI trigger for it, you can see it happening on ubuntu-latest here:

https://github.com/charlesnicholson/deptry-pep420-repro/actions/runs/9810619739/job/27091137978
Screenshot 2024-07-05 at 11 35 51 AM

@mkniewallner
Copy link
Collaborator

(closed by mistake by linking a PR to this issue)

@kojiromike
Copy link

I'm sad that #753 was reverted. Deptry has never been anything close to our slowest performer in our CI suites so we wouldn't have minded if the performance had declined even quite a bit. Plus, all our codebases are small. Would it be possible to make it enabled by configuration, but disabled by default?

@mkniewallner
Copy link
Collaborator

Support for PEP 420 has been added back in 0.19.1 behind --experimental-namespace-package feature flag (and the corresponding experimental_namespace_package option under [tool.deptry] in pyproject.toml).

Performance should be slightly better as we now do an early return if we find at least one Python file in the directory. There are some possible future improvements listed in #808 to improve performance even more.

@charlesnicholson
Copy link
Author

Hmm, I'm still seeing the error in my little repro testbed on v0.19.1:
Screenshot 2024-08-11 at 8 30 12 PM

The failed action is here:
https://github.com/charlesnicholson/deptry-pep420-repro/actions/runs/10343972148/job/28628860626

and the repro, updated to use --experimental-namespace-package is here:
https://github.com/charlesnicholson/deptry-pep420-repro

(Is my example incorrect?)

@mkniewallner
Copy link
Collaborator

Hmm, I'm still seeing the error in my little repro testbed on v0.19.1: Screenshot 2024-08-11 at 8 30 12 PM

The failed action is here: https://github.com/charlesnicholson/deptry-pep420-repro/actions/runs/10343972148/job/28628860626

and the repro, updated to use --experimental-namespace-package is here: https://github.com/charlesnicholson/deptry-pep420-repro

(Is my example incorrect?)

Yes sorry, nothing has changed since #740 (comment) for now, we've just reverted the feature because of a performance regression, then put it back behind a feature flag, but as is, meaning it doesn't fully fix your use case. I did not yet take the time to fully dig into it, but still planned at some point.

@javiatar-malted
Copy link

javiatar-malted commented Sep 16, 2024

We're also hitting this issue where, like @charlesnicholson we use the name of our company as a namespace package to avoid clashes with other third-party packages.

Problem

The current version (peptry 0.20.0) isn't able to properly recognise these dependencies (or indeed allow us to tell it what the imports are via package_module_name_map).

Indeed, if you have the following in example.py

import company.pkg1
import company.pkg2

with the following in pyproject.toml

dependencies = [
"company-pkg1",
"company-pkg2"
]

When running deptry you get: DEP002: 'company-pkg2' defined as a dependency but not used in the codebase(which is clearly false as it's in my second import in example.py).

Indeed if you run it with the --verbose flag you see the following:

Dependency 'company-pkg1' with top-levels: {'company'}.
Dependency 'company-pkg2' with top-levels: set().

And there's nothing you can do via config. For example use package_module_name_map like so:

[tool.deptry.package_module_name_map]
company-pkg1 = "company.pkg1"
company-pkg2 = "company.pkg2"

as you now get DEP002s for company-pkg1, and company-pkg2 in the pyproject.toml, and DEP001s for each import company.pkg# in the source code.

Solution

Ideally you'd add support for importing packages following PEP420 by not assuming that the top-most 'module' (or package), i.e 'company' in this example, is the dependency you're trying to import. Instead it should also check for matches of package.module for dependencies of the form namespacePackage-package in the pyproject.toml

Alternatively, just allow one to set a package_module_name_map that correctly identifies namespacePackage.package as in my example above.

Thanks in advance for taking a look at this, and for an otherwise great dependency management tool!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants