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

Rye lint command emits redundant output in workspaces #973

Open
cnpryer opened this issue Apr 1, 2024 · 3 comments
Open

Rye lint command emits redundant output in workspaces #973

cnpryer opened this issue Apr 1, 2024 · 3 comments

Comments

@cnpryer
Copy link
Contributor

cnpryer commented Apr 1, 2024

Steps to Reproduce

See this repo for a MWE https://github.com/cnpryer/rye-lint-mwe or use the following steps to reproduce it yourself:

rye init workspace --virtual
cd worksapce && rye init subproject
vim subproject/src/subproject/__init__.py

Add the following import:

import missing

Add the subproject to the pyproject workspace members

[tool.rye.workspace]
members = [
    "subproject"
]
rye lint --all
subproject/src/subproject/__init__.py:1:8: F401 [*] `missing` imported but unused
subproject/src/subproject/__init__.py:1:8: F401 [*] `missing` imported but unused
Found 2 errors.
[*] 2 fixable with the `--fix` option.

With the member excluded:

[tool.rye.workspace]
members = [
    # "subproject"
]
rye lint --all
subproject/src/subproject/__init__.py:1:8: F401 [*] `missing` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

Expected Result

It should emit 1 error.

Actual Result

It emits 2 errors.

Version Info

rye 0.33.0
commit: 0.32.0+13 (c25e748f0 2024-03-30)
platform: macos (aarch64)
self-python: [email protected]
symlink support: true
uv enabled: true

Stacktrace

No response

@cnpryer
Copy link
Contributor Author

cnpryer commented Apr 1, 2024

If we are including the root pyproject in ruff check aren't we running ruff on all sub-directories anyway?

I only skimmed Rye's lint/workspace logic, but isn't it effectively running this?

rye-lint-mwe on  main is 📦 v0.1.0 via 🐍 v3.12.2 
❯ ruff check . subproject
subproject/src/subproject/__init__.py:1:8: F401 `missing` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
subproject/src/subproject/__init__.py:1:8: F401 `missing` imported but unused; consider removing, adding to `__all__`, or using a redundant alias
Found 2 errors.

So if we find that the current directory is one of the projects why don't we collapse the command to it?

Edit: Or rather just collapse the paths into common roots.

@cnpryer
Copy link
Contributor Author

cnpryer commented Apr 1, 2024

I've PR'd what might resolve rye lint --all, but it needs to be improved.

If anyone is interested in this feel free to beat me to it. I'm not sure when I'll be able to pick this back up. My time can open up randomly. If it does I'll try to finish it out.

It'd be nice to add some tests as well.

@hyyking
Copy link
Contributor

hyyking commented Apr 6, 2024

This is a similar issue to #858 where pytest is run at the root collecting anything it sees I proposed that we mimic cargo in the workspace case meaning the command should not run the root but each sub package individually.

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 a pull request may close this issue.

2 participants