Skip to content

Conversation

@amotl
Copy link
Collaborator

@amotl amotl commented Oct 24, 2024

About

Add best-practice configuration for sandbox tasks to pyproject.toml, using poethepoet.

Details

The fundamental commands to use mostly, are:

  • poe format
  • poe check

Later, there will also be another fundamental command:

  • poe release

Other than those, there are more supportive commands, also to build Sphinx documentation and such.

@coderabbitai
Copy link

coderabbitai bot commented Oct 24, 2024

Warning

Rate limit exceeded

@amotl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 25 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Files that changed from the base of the PR and between 985d72e and 7df6d17.

Walkthrough

The changes in this pull request involve updates to documentation and configuration files related to the development environment of the project. A new "Development Sandbox" section has been added to DEVELOP.md, providing setup instructions. Additionally, a "Development" section has been introduced in README.md, referencing the new documentation. The requirements-dev.txt file has been removed, which previously listed development dependencies. Finally, the setup.py file has been modified to remove the UploadCommand class and update the extras_require section to include new optional dependencies for development and release.

Changes

File Change Summary
DEVELOP.md Added new section "Development Sandbox" with setup instructions for the development environment.
README.md Added new section "Development" referencing the "Development Sandbox" document.
requirements-dev.txt Removed the file, eliminating the specification of development dependencies.
setup.py Removed UploadCommand class; updated extras_require to add new optional dependencies for development and release; modified cmdclass to remove the upload command.

Poem

In the sandbox where we play,
Developers work day by day.
With commands to clone and test,
Our code will surely be the best!
No more uploads, just a clean slate,
Hopping to success, oh, isn’t it great? 🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@amotl amotl changed the title Tasks: Add configuration to pyproject.toml Sandbox: Add tasks configuration to pyproject.toml Oct 24, 2024
Base automatically changed from format to main October 24, 2024 11:30
@amotl amotl requested a review from kennethreitz October 24, 2024 13:50
@amotl amotl marked this pull request as ready for review October 24, 2024 13:50
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (3)
DEVELOP.md (3)

1-5: Enhance documentation clarity and completeness

Consider improving the documentation structure by:

  1. Adding a brief explanation of what a "development sandbox" is and its purpose
  2. Including prerequisites (required Python version, git, etc.)
  3. Adding a table of contents for better navigation

Example addition:

 # Development Sandbox
 
-Set up a development sandbox.
+This guide helps you set up a local development environment for contributing to the Responder framework.
+
+## Prerequisites
+- Python 3.7 or higher
+- Git
+
+## Table of Contents
+- [Installation](#installation)
+- [Development Commands](#development-commands)

6-12: Clarify installation options and add verification steps

The installation command includes multiple extras without explanation. Consider:

  1. Documenting what each extra provides (graphql, develop, release, test)
  2. Adding verification steps to confirm successful installation

Example improvement:

 Acquire sources and install project in editable mode.
 ```shell
 git clone https://github.com/kennethreitz/responder
 cd responder
 python3 -m venv .venv
 source .venv/bin/activate
 pip install --editable '.[graphql,develop,release,test]'
+
+# Verify installation
+python -c "import responder; print(responder.__version__)"

+The installation includes the following optional dependencies:
+- graphql: GraphQL support
+- develop: Development tools (linting, formatting)
+- release: Package release utilities
+- test: Testing frameworks and dependencies


---

`14-22`: **Document development commands and their configuration**

The commands section needs more context about:
1. What `poe` is and its relationship to poethepoet
2. What each command does specifically
3. Where these commands are configured


Example improvement:
```diff
-Invoke linter and software tests.
+## Development Commands
+
+The project uses [poethepoet](https://github.com/nat-n/poethepoet) for task management, configured in `pyproject.toml`.
+
+Run linting checks and tests:
 ```shell
 poe check

+This command:
+- Runs type checking with mypy
+- Executes linting with flake8
+- Runs unit tests with pytest

-Format code.
+Format code according to project standards:

poe format

+This command:
+- Formats code using black
+- Sorts imports using isort


</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**

<details>
<summary>📥 Commits</summary>

Files that changed from the base of the PR and between b39c539d576a4c3949d5692703aba7d3781bcf89 and 985d72e74be3b7f00b7bbebd4df511353e1ac36f.

</details>

<details>
<summary>⛔ Files ignored due to path filters (2)</summary>

* `.github/workflows/test.yaml` is excluded by `!**/*.yaml`
* `pyproject.toml` is excluded by `!**/*.toml`

</details>

<details>
<summary>📒 Files selected for processing (4)</summary>

* DEVELOP.md (1 hunks)
* README.md (1 hunks)
* requirements-dev.txt (0 hunks)
* setup.py (3 hunks)

</details>

<details>
<summary>💤 Files with no reviewable changes (1)</summary>

* requirements-dev.txt

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>

* README.md

</details>

<details>
<summary>🔇 Additional comments (3)</summary><blockquote>

<details>
<summary>setup.py (3)</summary><blockquote>

`35-35`: **LGTM! Version constraint maintained.**

The version constraint for typesystem remains appropriate to prevent breaking changes.

---

Line range hint `1-111`: **Verify removal of upload command references.**

The migration from setup.py commands to pyproject.toml tasks is a good modernization step. Let's ensure there are no remaining references to the old upload command.

```shell
#!/bin/bash
# Description: Check for any remaining references to the old upload command
echo "Checking for references to setup.py upload command..."
rg -l "setup.py.*upload" --type md --type txt --type rst

89-91: Well-structured modernization of development tooling!

The new development and release extras introduce modern, recommended tools that align with Python best practices. The poethepoet addition supports the PR's objective of task configuration.

Let's verify these are the current recommended tools:

✅ Verification successful

Since the PyPI API calls didn't work in the sandbox environment, let's try a different approach to verify these tools by checking their usage patterns in the codebase and related configuration files.

This will help us understand:

  1. If these tools are properly configured in the project
  2. If there are any existing task runner patterns that poethepoet would replace
  3. If the build system is properly specified for using these tools

Let's examine the contents of pyproject.toml since it appears to contain configurations for these tools.

This will help us verify:

  1. If the new tools are properly configured in pyproject.toml
  2. What existing functionality these tools are replacing
  3. If the build system and tool configurations align with the extras being added

The new development tools are well-configured and actively used in the project

The verification shows that:

  1. All new tools are properly configured and integrated:
    • ruff has detailed configuration for linting and formatting
    • poethepoet has comprehensive task definitions including lint, test, format, and release tasks
    • pyproject-fmt is used in the format task
    • validate-pyproject is part of the lint task
    • build and twine are configured in the release task
  2. These tools are replacing a simpler custom DebCommand, modernizing the project's development workflow
  3. The tools are actively used with proper configurations, showing they're not just dependencies but integral parts of the development process
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if these are the latest recommended tools by examining the project's PyPI stats
echo "Checking PyPI download stats and latest versions for the new tools..."
for pkg in poethepoet pyproject-fmt ruff validate-pyproject build twine; do
    echo "=== $pkg ==="
    # Get latest version and weekly downloads
    curl -s "https://api.pypistats.org/packages/$pkg/recent" | jq '.data.last_week'
    curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.version'
done

Length of output: 373


Script:

#!/bin/bash
# Check for tool configurations and usage patterns
echo "=== Checking for tool configurations ==="
# Look for pyproject.toml which typically contains modern Python tool configs
fd pyproject.toml

# Check for ruff configuration
fd '\.ruff\.toml|ruff\.toml'
fd --type f -e toml -x grep -l 'tool.ruff' {} \;

# Look for poe (poethepoet) task definitions
fd --type f -e toml -x grep -l '\[tool.poe.tasks\]' {} \;

# Check for any build system specifications
fd --type f -e toml -x grep -l 'build-system' {} \;

echo -e "\n=== Checking imports and usage patterns ==="
# Check for imports/usage of these tools in Python files
rg -l "from (poethepoet|ruff|twine|build|validate_pyproject)" --type py

# Look for any existing task runner patterns
rg -l "task_|tasks\." --type py

Length of output: 588


Script:

#!/bin/bash
# Examine pyproject.toml contents
echo "=== Checking pyproject.toml configuration ==="
cat pyproject.toml

# Also check for any existing setup.py task runners or commands that might be replaced
echo -e "\n=== Checking setup.py for existing commands ==="
rg -A 5 "class.*Command" setup.py

Length of output: 2677

@amotl amotl requested a review from tabotkevin October 24, 2024 16:19
@amotl amotl force-pushed the tasks branch 2 times, most recently from 97c8ba9 to 86456fc Compare October 24, 2024 16:24
The fundamental commands to mostly use are:

- poe format
- poe check
@kennethreitz
Copy link
Owner

kennethreitz commented Oct 24, 2024

can we use 'uv' for checks?

@tabotkevin
Copy link
Collaborator

@amotl I’m leaning towards using uv or rye for formatting, linting, and checks, with ruff as the underlying tool. I think the entire Python ecosystem is shifting towards uv. Dyne currently uses rye, and it’s been a breeze to work with.

@amotl
Copy link
Collaborator Author

amotl commented Oct 25, 2024

Hi there. Thanks for your replies.

If you agree in progressively modernizing the project metadata setup towards pyproject.toml, and if you don't have any objections about bringing in and learning about the poethepot task runner, let's bring in this patch, and then adjust its incantation commands towards using uv and rye on behalf of subsequent iterations?

I will be very happy to learn about them from your expertise, as a few friends also started using them, and I am a bit behind, just being a user of ruff up until now.

Dyne currently uses rye, and it’s been a breeze to work with.

Excellent. Would you be able to carry over relevant advises here, or should I just cherry-pick from there on the next iteration?

@amotl
Copy link
Collaborator Author

amotl commented Oct 25, 2024

Using uv

can we use 'uv' for checks?

Because I never used uv before, I would need more guidance.

I will be very happy to learn about uv and rye from your expertise, as a few friends also started using them, and I am a bit behind, just being a user of ruff up until now.

Problem?

In the current state, we apparently can't do much with uv run, if that is what you are referring to?

$ uv run -- echo hello
error: No `project` table found in: `/path/to/responder/pyproject.toml`

The same also happens with other subcommands:

$ uv tree
error: No `project` table found in: `/path/to/responder/pyproject.toml`

I am using:

$ uv version
uv 0.4.3 (Homebrew 2024-09-02)

Evaluation

The migration of project metadata to pyproject.toml would probably need to proceed further, which I don't favor to also add to this patch.

@tabotkevin
Copy link
Collaborator

Here's a revised version:

@amotl, to provide some context: rye was initially intended as the main tool, planned to work with uv as a foundation. However, uv gradually started covering all of rye's intended functionality, leading to the decision to prioritize uv over rye. The error you're encountering is likely because the current project wasn't created with uv. To fully transition the project to use uv, you'll need to create a new uv project and move all the files into the structure that uv sets up. This is the approach I used when I migrated Dyne to rye.

@kennethreitz
Copy link
Owner

Yeah, I see little reason for us to use anything but uv. I can play with it in the future, i'm glad we're in a better spot now.

@kennethreitz kennethreitz merged commit 413028b into main Oct 25, 2024
12 checks passed
@kennethreitz kennethreitz deleted the tasks branch October 25, 2024 11:39
@kennethreitz
Copy link
Owner

@coderabbitai can you tell us how rye shouldn't be necessary with recent uv developments?

@coderabbitai
Copy link

coderabbitai bot commented Oct 25, 2024

Tip

For best results, initiate chat on the files or code changes.

@kennethreitz: Certainly! With recent developments, uv has expanded to include much of the functionality that rye previously provided. Originally, rye was intended to work alongside uv as the main tool for managing development tasks. However, uv has evolved to encompass features like task management, environment setup, formatting, linting, and checks—all within a single tool. This means we can now rely solely on uv for our development workflow, making rye unnecessary and simplifying our toolchain.

@amotl
Copy link
Collaborator Author

amotl commented Oct 25, 2024

uv has evolved to encompass features like task management [amongst others].

I see. I am excited to see if this feature of uv can make poethepoet obsolete. Thanks for merging.

@kennethreitz
Copy link
Owner

No problem!

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.

4 participants