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.

Sign up for GitHub

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

Jump to bottom

Support PDM through pyproject.toml #114

Closed
sanmai-NL opened this issue Feb 11, 2023 · 11 comments
Closed

Support PDM through pyproject.toml #114

sanmai-NL opened this issue Feb 11, 2023 · 11 comments
Assignees
@fredrikaverpil

Comments

@sanmai-NL
Copy link
Contributor

sanmai-NL commented Feb 11, 2023 •
edited
Loading

Problem

When using PDM and not Poetry, creosote 1.0.3 fails:

Parsing pyproject.toml for packages
Traceback (most recent call last):
  File "/package/.venv/lib/python3.10/site-packages/creosote/parsers.py", line 24, in _pyproject
    deps = contents["tool"]["poetry"]["dependencies"]
KeyError: 'poetry'

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/package/.venv/bin/creosote", line 8, in <module>
    sys.exit(main())
  File "/package/.venv/lib/python3.10/site-packages/creosote/cli.py", line 84, in main
    deps_reader.read(args.deps_file, args.dev)
  File "/package/.venv/lib/python3.10/site-packages/creosote/parsers.py", line 58, in read
    self.packages = self.packages_sans_ignored(self._pyproject(deps_file, dev))
  File "/package/.venv/lib/python3.10/site-packages/creosote/parsers.py", line 26, in _pyproject
    raise Exception("Could not find expected toml property.") from e
Exception: Could not find expected toml property.

Sketch of solution

Iterate over project.dependencies and the keys under tool.pdm.dev-dependencies in pyproject.toml.
@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Feb 13, 2023 •
edited
Loading

Hi @sanmai-NL and thanks for reporting.

Right now the parser is just looking for pre-defined toml sections in the pyproject.toml. I've been meaning to rewrite this part of the tool as it does not e.g. support PEP 621 or Poetry dependency groups.

I've not used PDM, but after having a quick glance in the docs, it looks like it's using PEP 621 (which is great) plus a [tool.pdm.dev-dependencies] section, just like you mentioned.

One thing to start with here is to play around with extending this part of the code:
https://github.com/fredrikaverpil/creosote/blob/main/src/creosote/parsers.py#L20-L26

🤔 I'm wondering if there's a more clever way to fetch dependencies from the build backend.

Feel free to play around and open a PR if you wish. I'll try to look into this, but not sure when I'll have the time at the moment.

@fredrikaverpil fredrikaverpil mentioned this issue Mar 4, 2023
Merged
@fredrikaverpil
Copy link
Owner

Hi again @sanmai-NL, please try creosote v2.0.0. It how should support PDM. Let me know if you experience any issues!

@sanmai-NL
Copy link
Contributor Author

I'm not sure whether this fault has to do with the PDM support. It looks like a defect parsing the astroid 2.15.0 source code (dependency of pylint 2.16.3).

$  creosote -v .venv -p .
Parsing __version__.py
Parsing core/__init__.py
Parsing core/__main__.py
Parsing componentresources/__init__.py
Parsing componentresources/datapipelinesegment/__init__.py
Parsing componentresources/datapipelinesegment/__version__.py
Parsing componentresources/datapipelinesegment/.venv/bin/activate_this.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/semver.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/test_autoflake.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/bugbear.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/mccabe.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/_black_version.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/_virtualenv.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/pycodestyle.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/mypy_extensions.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/six.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/nodeenv.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/cfgv.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/py.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/autoflake.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/typing_extensions.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/autopep8.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/_backport_stdlib_names.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/constraint.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/_cache.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/transforms.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/test_utils.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/mixins.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/protocols.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/util.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/arguments.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/modutils.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/__init__.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/raw_building.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/_ast.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/scoped_nodes.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/inference_tip.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/builder.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/__pkginfo__.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/astroid_manager.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/context.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/rebuilder.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/inference.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/exceptions.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/objects.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/typing.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/helpers.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/node_classes.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/manager.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/filter_statements.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/const.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/bases.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/decorators.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/dunder_lookup.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/__init__.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/objectmodel.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/_import/util.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/_import/__init__.py
Parsing componentresources/datapipelinesegment/.venv/lib/python3.11/site-packages/astroid/interpreter/_import/spec.py
Traceback (most recent call last):
  File "/Users/sanderhan/devel/gitlab.com/han-aim/research/instrumentenmakerij/infra/.venv/bin/creosote", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/Users/sanderhan/devel/gitlab.com/han-aim/research/instrumentenmakerij/infra/.venv/lib/python3.11/site-packages/creosote/cli.py", line 77, in main
    imports = parsers.get_modules_from_code(args.paths)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sanderhan/devel/gitlab.com/han-aim/research/instrumentenmakerij/infra/.venv/lib/python3.11/site-packages/creosote/parsers.py", line 171, in get_modules_from_code
    for imp in get_module_info_from_code(resolved_path):
  File "/Users/sanderhan/devel/gitlab.com/han-aim/research/instrumentenmakerij/infra/.venv/lib/python3.11/site-packages/creosote/parsers.py", line 151, in get_module_info_from_code
    module = node.module.split(".")
             ^^^^^^^^^^^^^^^^^

@sanmai-NL sanmai-NL mentioned this issue Mar 6, 2023
Closed
@fredrikaverpil
Copy link
Owner

@sanmai-NL great find. This is also reproducible on my end. Will look into it.

@fredrikaverpil fredrikaverpil self-assigned this Mar 6, 2023
@fredrikaverpil fredrikaverpil mentioned this issue Mar 6, 2023
Merged
@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Mar 6, 2023 •
edited
Loading

I can no longer reproduce the bug with the just released v2.1.1. Please try it out and let me know how it goes.

@sanmai-NL
Copy link
Contributor Author

The fault is gone, but parsing a product with a virtual environment with a few very source code-heavy packages takes long (a quarter, maybe longer, didn't time it), and the next phase then seems to stall. These source code-heavy packages contain generated Python source code to interact with large web APIs like Azure Resource Manager.

Trying it on a smaller component does yield the correct (production not, dev) dependencies, but then these are all incorrectly reported as unused.

$ creosote -v .venv -p core
Parsing core/__init__.py
Parsing core/__main__.py
Parsing pyproject.toml for packages
Found packages in pyproject.toml: nsone, pip, pulumi, pulumi-azure-native, pulumi-command, pulumi-kubernetes, pulumi-ns1, setuptools
Resolving...
Unused packages found: nsone, pip, pulumi-azure-native, pulumi-command, pulumi-ns1, setuptools

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Mar 6, 2023 •
edited
Loading

I'm not sure why you run the tool with -p .. You're supposed to point -p to your own source code only; either folder(s) and/or file(s). It's, however, curious to hear it's so slow.

Just for context, the -v could've been optional but might give you the wrong results (as package name vs imported name might differ).

In your smaller example with -p core, are you sure the packages reported as unused are truly imported from the files in the "core" folder?

@sanmai-NL
Copy link
Contributor Author

I'm used to tools supporting a recursive scan for non-ignored Python source code. My root pyproject.toml root directory isn't a single package but akin to a workspace. The subdirectories of componentresources do have their own pyproject.toml files. I think these should be excluded to prevent inconsistencies, even if specified with -p.

$ tree -a -h -L 2 -P '*.py' -P '*.toml'
[1.0K]  .
├── [ 480]  .git
│   ├── [ 480]  hooks
│   ├── [  96]  info
│   ├── [ 128]  logs
│   ├── [  96]  modules
│   ├── [6.3K]  objects
│   └── [ 192]  refs
├── [ 192]  .mypy_cache
│   ├── [3.9K]  3.10
│   └── [2.7K]  3.11
├── [ 112]  .pdm.toml
├── [ 160]  .ruff_cache
│   └── [2.0K]  content
├── [ 192]  .venv
│   ├── [1.6K]  bin
│   └── [  96]  lib
├── [ 128]  .vscode
├── [  22]  __version__.py
├── [ 384]  azure
│   ├── [ 959]  __init__.py
│   ├── [ 343]  __main__.py
│   ├── [ 224]  __pycache__
│   ├── [2.8K]  aks.py
│   ├── [ 224]  data
│   └── [ 40K]  taloslinux.py
├── [  64]  build
├── [ 256]  componentresources
│   ├── [  22]  __init__.py
│   ├── [ 128]  __pycache__
│   ├── [ 384]  datapipelinesegment
│   ├── [ 352]  staticwebsite
│   └── [ 384]  webserver
├── [ 320]  core
│   ├── [  22]  __init__.py
│   ├── [ 985]  __main__.py
│   └── [ 128]  __pycache__
├── [ 448]  datapipeline_ff
│   ├── [  96]  .vscode
│   ├── [  22]  __init__.py
│   ├── [ 814]  __main__.py
│   └── [ 160]  __pycache__
├── [  64]  environments
├── [2.3K]  pyproject.toml
└── [ 704]  template_Python
    ├── [  96]  .vscode
    ├── [  22]  __version__.py
    ├── [ 288]  k8s
    ├── [1.8K]  pyproject.toml
    ├── [  96]  src
    └── [ 224]  template_Kubernetes

37 directories, 14 files

For some reason, creosote -v .venv -p datapipeline_ff/ core/ componentresources/ azure/ parses all source code under .venv/ too. That seems incorrect?

@fredrikaverpil
Copy link
Owner

fredrikaverpil commented Mar 7, 2023 •
edited
Loading

@sanmai-NL I see. 🤔 I'm thinking of sane defaults and also providing the means to use creosote easily in your situation.

For some reason, creosote -v .venv -p datapipeline_ff/ core/ componentresources/ azure/ parses all source code under .venv/ too. That seems incorrect?

Indeed, that sounds wrong. Let me see if I can reproduce it.

But I'm also wondering about the unused packages you saw in your core. Are you sure those packages were indeed imported in the files within core?

@sanmai-NL
Copy link
Contributor Author

While some dependencies are only used elsewhere in the workspace, core/__main__.py contains at the top level:

from pulumi import Config, ResourceOptions, export, get_project
from pulumi_kubernetes import Provider

@fredrikaverpil fredrikaverpil mentioned this issue Mar 10, 2023
Closed
@fredrikaverpil
Copy link
Owner

Since the original issue of PDM was solved, I'll close this issue. Let's continue the conversation over at #128 about the problems you are seeing in your workspace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fredrikaverpil fredrikaverpil

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fredrikaverpil @sanmai-NL