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

Use uv workspaces instead of pip #12622

Closed
wants to merge 5 commits into from
Closed

Use uv workspaces instead of pip #12622

wants to merge 5 commits into from

Conversation

MichaReiser
Copy link
Member

Summary

I don't know what I'm doing and this doesn't work.

  • The lock file looks okay?
  • I can't run any scripts because the dependencies don't seem to get installed
  • I'm probably doing something very wrong

Test Plan

See contributing.md. I didn't get passed uv sync

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Aug 2, 2024
docs/pyproject.toml Outdated Show resolved Hide resolved
@@ -1,8 +1 @@
PyYAML==6.0.1
black==23.10.0
mkdocs==1.5.0
mkdocs-material @ git+ssh://git@github.com/astral-sh/mkdocs-material-insiders.git@38c0b8187325c3bab386b666daf3518ac036f2f4
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to keep it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use a custom version of mkdocs but external contributors don't have access to that repository. I don't know if there's a better way to support this?

Co-authored-by: T-256 <132141463+T-256@users.noreply.github.com>
@zanieb
Copy link
Member

zanieb commented Aug 2, 2024

Can you clarify what's not working?

@MichaReiser
Copy link
Member Author

I can't run the mkdocs script because the dependencies are missing (I checked, they're not in my venv)

@charliermarsh
Copy link
Member

I'm not sure what you're running exactly but we need to mark Ruff as unmanaged. We don't want to be building Ruff to run scripts.

@MichaReiser
Copy link
Member Author

I try to run uv run --verbose scripts/generate_mkdocs.py. Overall, I try to get the workflow documented in the contributing.md to work.

@charliermarsh
Copy link
Member

It's because the workspace members aren't declared as dependencies of the project.

@charliermarsh
Copy link
Member

uv run --package ruff-docs --verbose scripts/generate_mkdocs.py does the right thing, but fails because ./docs isn't a valid Python package (it needs src/ruff_docs/__init__.py).

@charliermarsh
Copy link
Member

If you add this to docs/pyproject.toml, then uv run --package ruff-docs -- scripts/generate_mkdocs.py works:

[tool.hatch.build.targets.wheel]
packages = ["src/ruff_docs"]

I think we should re-add that to uv init, it got dropped during code review.

@@ -161,7 +161,7 @@ def main() -> None:
# Format rules docs
add_no_escape_text_plugin()
for rule_doc in Path("docs/rules").glob("*.md"):
mdformat.file(rule_doc, extensions=["mkdocs", "admonition", "no-escape-text"])
mdformat.file(rule_doc, extensions=["mkdocs", "admon", "no-escape-text"])
Copy link
Member

Choose a reason for hiding this comment

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

This was a breaking change in mdformat-admon, in the 2.0.3 release: KyleKing/mdformat-mkdocs@6a32997

@@ -294,28 +294,30 @@ Finally, regenerate the documentation and generated code with `cargo dev generat

To preview any changes to the documentation locally:

1. Install the [Rust toolchain](https://www.rust-lang.org/tools/install).
1. Install the [Rust toolchain](https://www.rust-lang.org/tools/install).]
Copy link
Member

Choose a reason for hiding this comment

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

This seems off... Isn't that mismatched now?

@MichaReiser
Copy link
Member Author

Okay, I don't understand why I would have to declare ruff_docs as a package just so that the generate script can resolve mdformat 😕

Copy link
Contributor

github-actions bot commented Aug 2, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

warning: Detected debug build without --no-cache.
error: Failed to parse examples/Chat_finetuning_data_prep.ipynb:6:18:25: Unparenthesized generator expression cannot be used here
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_confluence.ipynb:15:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_gmail.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_jira.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_notion.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_doc.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_text.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_middleware_azure_function.ipynb:37:1:13: Simple statements must be separated by newlines or semicolons

Formatter (preview)

ℹ️ ecosystem check encountered format errors. (no format changes; 1 project error)

openai/openai-cookbook (error)

ruff format --preview

warning: Detected debug build without --no-cache.
error: Failed to parse examples/Chat_finetuning_data_prep.ipynb:6:18:25: Unparenthesized generator expression cannot be used here
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_confluence.ipynb:15:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_gmail.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_jira.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_notion.ipynb:15:1:1: Expected an expression
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_doc.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_action_sharepoint_text.ipynb:28:1:5: Simple statements must be separated by newlines or semicolons
error: Failed to parse examples/chatgpt/gpt_actions_library/gpt_middleware_azure_function.ipynb:37:1:13: Simple statements must be separated by newlines or semicolons

@charliermarsh
Copy link
Member

Can you expand on your question?

@MichaReiser
Copy link
Member Author

It's more of an observation. I understand that the packages setting makes the listed package available to other packages. But the generate_mkdocs script doesn't depend on the ruff_docs package. It depends on some of its transitive dependencies.

That's why I think it odd that I have to add ruff_docs to packages just to make the script work. I could understand if I have to move the script into the ruff_docs directory to make it work (so that uv knows that it should run the script with additional third party dependencies).

@charliermarsh
Copy link
Member

charliermarsh commented Aug 5, 2024

@MichaReiser - I think part of the problem is that ruff_docs isn't really a package... An alternative is to make these all dev dependencies in the workspace root. Do you find that more intuitive?

diff --git a/pyproject.toml b/pyproject.toml
index eeba1c160..1ab117188 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -8,7 +8,7 @@ version = "0.5.6"
 description = "An extremely fast Python linter and code formatter, written in Rust."
 authors = [{ name = "Astral Software Inc.", email = "hey@astral.sh" }]
 readme = "README.md"
-requires-python = ">=3.7"
+requires-python = ">=3.8"
 license = { file = "LICENSE" }
 keywords = [
   "automation",
@@ -44,7 +44,18 @@ Documentation = "https://docs.astral.sh/ruff/"
 Changelog = "https://github.com/astral-sh/ruff/blob/main/CHANGELOG.md"

 [tool.uv.workspace]
-members = ["docs", "python/ruff-ecosystem"]
+members = ["python/ruff-ecosystem"]
+
+[tool.uv]
+dev-dependencies = [
+  "PyYAML>=6.0.1",
+  "mkdocs>=1.5.0",
+  "mdformat>=0.7.17",
+  "mdformat-mkdocs>=2.0.4",
+  "mdformat-admon>=2.0.3",
+  "mkdocs-material>=9.1.18",
+  "mkdocs-redirects>=1.2.1",
+]

 [tool.maturin]
 bindings = "bin"

Then uv run -- scripts/generate_mkdocs.py works.

(I initially didn't realize that the script you were running wasn't part of docs.)

@T-256
Copy link
Contributor

T-256 commented Aug 5, 2024

Can I work on this?

@MichaReiser
Copy link
Member Author

MichaReiser commented Aug 6, 2024

@charliermarsh I see. But let's pretend that ruff_docs is a package.

An alternative is to make these all dev dependencies in the workspace root. Do you find that more intuitive?

Kind of. With lerna I would have two options:

  1. Make it a workspace-level script
  2. Move it into ruff_docs and make it a ruff_docs script

To me it seems uv only supports 1. or what's the recommended way to run generate_mkdocs.py when declaring the dependency on the package level (let's pretend that ruff_docs is a real package)

@T-256 sure! Feel free to open a new PR. I then close this one

@charliermarsh
Copy link
Member

@MichaReiser -- Genuinely trying to follow but I'm a little confused. For (2), you can add a pyproject.toml to ruff_docs, declare the dependencies in that pyproject.toml, and then run uv run --package ruff-docs ./scripts/generate_mkdocs.py. Isn't that the current state of this PR?

@MichaReiser
Copy link
Member Author

Isn't that the current state of this PR?

Not what's described in the CONTRIBUTING.md. So no. I wasn't aware that this is a possibility.

I think where the behavior of uv run differs from what I expected is that I expected uv run ruff_docs/generate_mkdocs to have the same behavior as uv run -p ruff_docs ruff_docs/generate_mkdocs

@charliermarsh
Copy link
Member

Oh I see, that running a script in the ruff_docs directory would behave equivalently to running a script from the ruff_docs directory.

@zanieb
Copy link
Member

zanieb commented Aug 6, 2024

Fwiw we do uvx --with-requirements docs/requirements.txt -- mkdocs serve -f mkdocs.public.yml in uv (but I know it's a little different over here)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants