feat: Use Pixi, pyradiomics fork #40
Conversation
…sic config files for coverage and linting
…te readii version to 1.11.0 with new author email
…-release task, and update pixi.lock
WalkthroughThe changes involve a comprehensive update to the CI/CD workflow, transitioning from a Poetry-based to a Pixi-based configuration. The workflow now includes expanded triggers, matrix strategies for testing across multiple environments, and a focus on using Pixi for dependency management. Several jobs have been restructured or commented out, reflecting a shift in the build and release process. Additionally, new configuration files for linting and coverage have been introduced, and the Changes
Poem
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (11)
config/hatch.toml (1)
1-5: Overall, the Hatch configuration looks good.This new
hatch.tomlfile establishes a clear build configuration for the project, which is crucial for the transition to a Pixi-based setup. It defines the build output directory and specifies the package to be included in the wheel. These changes should improve the project's build process and align it with modern Python project management practices.However, please ensure that the relative path "../src/readii" is correct in relation to where this config file is located in the project structure. If the path is incorrect, it could lead to build failures.
Consider placing this configuration file in the project root to avoid potential path-related issues and to make the configuration more intuitive for other developers.
config/ruff.toml (2)
20-40: Comprehensive lint selection, but consider allowing removal of commented-out code.The selection of linting rules is thorough and covers various aspects of code quality, which is excellent. The decision to ignore "ANN101" (missing type annotation for self in method) is a common and acceptable practice.
However, preventing the automatic removal of commented-out code (ERA rules set as unfixable) might hinder code cleanliness in the long run. Commented-out code often becomes outdated and irrelevant over time.
Consider removing the ERA rule from the
unfixablelist to allow automatic cleanup of commented-out code:unfixable = [ - "ERA", # Don't remove commented-out code ]If you decide to keep this setting, it's recommended to periodically review and clean up commented-out code manually to maintain code quality.
59-66: Good docstring and formatting choices, but consider adjusting docstring code line length.The configuration makes good choices for docstring convention and string quoting:
- Using the Google convention for docstrings promotes consistency and readability.
- Preferring single quotes for strings is a common stylistic choice in Python.
However, the docstring code line length of 20 characters seems overly restrictive and might lead to excessive line breaks in code examples within docstrings.
Consider increasing the docstring code line length to a more comfortable value, perhaps matching or slightly less than the main line length:
[format] quote-style = "single" docstring-code-format = true -docstring-code-line-length = 20 +docstring-code-line-length = 88 # or another value that suits your projectold_pyproject.toml (2)
20-30: LGTM: Comprehensive development dependencies.The development dependencies cover essential areas including testing, documentation, and release management. The version constraints are appropriately set to allow for compatible updates.
For consistency, consider adding Python version constraints to other dependencies that might have specific Python version requirements, similar to the
myst-nbdependency.
32-42: Semantic release configuration looks good, with a note on patch releases.The semantic release configuration is well-structured and covers essential aspects of versioning and release management.
The
patch_without_tag = truesetting allows for patch releases without requiring a tag. While this can be convenient, it might lead to unexpected version bumps. Consider if this aligns with your versioning strategy and team workflows.pyproject.toml (3)
41-66: LGTM: Comprehensive test configuration with a suggestion.The test configuration is well-defined, including dependencies, pytest setup with coverage reporting, and a separate coverage task. This is a good setup for maintaining code quality.
Consider adding a minimum coverage threshold to ensure code quality standards are maintained. You can add this to your pytest command:
"pytest", "--numprocesses=auto", "-s", "--verbose", "--cov=readii", "--cov-report=xml:coverage-report/coverage.xml", "--cov-config=config/coverage.toml", + "--cov-fail-under=80",This will cause the tests to fail if coverage falls below 80% (adjust the percentage as needed).
68-85: TODO: Implement documentation and style check tasks.While the dependencies for documentation and style checking are correctly specified, the tasks for building documentation and running style checks are missing. These tasks are crucial for maintaining code quality and up-to-date documentation.
Would you like assistance in implementing these tasks? Here's a suggested implementation for the style check task:
[tool.pixi.feature.style.tasks.lint] cmd = "ruff check src tests" inputs = ["src", "tests", "config/ruff.toml"] description = "Run ruff linter" [tool.pixi.feature.style.tasks.format] cmd = "ruff format src tests" inputs = ["src", "tests", "config/ruff.toml"] description = "Run ruff formatter"For the documentation build task:
[tool.pixi.feature.docs.tasks.build-docs] cmd = "sphinx-build -b html docs/source docs/build" inputs = ["docs/source", "src"] outputs = ["docs/build"] description = "Build documentation using Sphinx"Let me know if you'd like me to implement these tasks or if you need any modifications.
Line range hint
119-129: Consider refining the branch matching pattern for semantic release.The semantic release configuration looks good overall, but the
branches.main.matchsetting might be too permissive:[tool.semantic_release.branches.main] match = "*"This could potentially trigger releases on unintended branches.
Consider using a more specific pattern to match only the main branch and potentially release branches. For example:
[tool.semantic_release.branches.main] match = "^main$" prerelease = false [tool.semantic_release.branches.release] match = "^release/" prerelease = trueThis configuration ensures that releases are only triggered on the main branch and release branches, providing better control over the release process.
.github/workflows/ci-cd.yml (2)
13-45: Approve Unit-Tests job with a suggestion.The Unit-Tests job has been significantly improved:
- Matrix strategy for multiple OS and Python versions enhances test coverage.
- Transition to Pixi aligns with the PR objectives.
However, consider the following suggestion:
The 15-minute timeout (line 18) might be too short for complex test suites. Consider increasing it to 30 minutes to prevent premature job termination:
- timeout-minutes: 15 # Consider increasing timeout + timeout-minutes: 30
97-147: Fix minor formatting issues.The static analysis tool reported some minor formatting issues:
- Trailing spaces on lines 97, 99, 101, 104, 110, and 144.
- Missing newline at the end of the file.
To resolve these, consider:
- Using a linter or EditorConfig to automatically trim trailing spaces.
- Ensuring your editor is configured to add a newline at the end of files.
These changes will improve the overall code quality and consistency.
🧰 Tools
🪛 yamllint
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 147-147: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/ci-cd.yml.disabled (1)
47-47: Remove extra '#' in commentThe comment line appears to have an extra '#' symbol, which may cause confusion.
Apply this diff to fix the comment:
- # # Only run this job if new work is pushed to "main" + # Only run this job if new work is pushed to "main"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
pixi.lockis excluded by!**/*.lockpoetry.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
- .github/workflows/ci-cd.yml (2 hunks)
- .github/workflows/ci-cd.yml.disabled (1 hunks)
- .gitignore (1 hunks)
- config/coverage.toml (1 hunks)
- config/hatch.toml (1 hunks)
- config/ruff.toml (1 hunks)
- old_pyproject.toml (1 hunks)
- pyproject.toml (2 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- config/coverage.toml
🧰 Additional context used
🪛 yamllint
.github/workflows/ci-cd.yml
[error] 97-97: trailing spaces
(trailing-spaces)
[error] 99-99: trailing spaces
(trailing-spaces)
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 104-104: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 147-147: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (17)
config/hatch.toml (2)
1-2: LGTM: Build configuration is well-defined.The [build] section correctly specifies "dist" as the output directory for build artifacts. This is a standard practice and aligns well with the project's transition to a Pixi-based setup.
4-5: Verify the relative package path.The [build.targets] section correctly defines a wheel target. However, the package location "../src/readii" is relative to this config file. Ensure that this path is correct relative to the project root.
Let's verify the project structure and package location:
✅ Verification successful
Verified the relative package path.
The
[build.targets]section correctly defines the wheel target with the relative path../src/readii, and thesrc/readiidirectory exists as expected relative toconfig/hatch.toml.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the project structure and package location # Test 1: Check if the config file is in a subdirectory if [ "$(dirname "$(pwd)")" != "$(git rev-parse --show-toplevel)" ]; then echo "Warning: config/hatch.toml is not in the project root." fi # Test 2: Verify the existence of the src/readii directory if [ -d "../src/readii" ]; then echo "src/readii directory exists relative to config/hatch.toml" else echo "Warning: src/readii directory not found relative to config/hatch.toml" fi # Test 3: List the content of the src directory for context echo "Content of the src directory:" ls -R ../srcLength of output: 552
Script:
#!/bin/bash # Description: Locate the src/readii directory within the repository # Search for the src/readii directory from the repository root fd -t d readii srcLength of output: 32
config/ruff.toml (3)
43-44: Appropriate exception for assert statements in test files.The configuration to ignore rule "S101" (which typically flags the use of assert statements) for test files is appropriate. Assert statements are common and often necessary in test code.
47-48: Appropriate maximum complexity setting.Setting the maximum McCabe complexity to 10 is a common and reasonable threshold. This helps maintain code readability and testability by encouraging the breakdown of complex functions.
51-57: Well-configured import sorting settings.The isort configuration promotes consistent and organized import statements:
- Specifying known first-party packages helps maintain clear separation between project and third-party imports.
- Forcing wrap aliases and combining as-imports enhances readability.
- Setting a line after imports improves code structure.
- Ordering relative imports from closest to furthest is intuitive and maintainable.
These settings align well with Python best practices for import organization.
old_pyproject.toml (3)
1-7: LGTM: Project metadata is well-defined.The project metadata is complete and follows best practices. The version number adheres to semantic versioning, and the MIT license is appropriate for open-source projects.
44-49: LGTM: Script entry point and build system configuration are appropriate.The script entry point for "readii" is well-defined, and the build system configuration is standard for a Poetry project. These sections are correctly set up for the package's build and execution.
9-18: Verify the stability of pyradiomics alpha version.The dependencies are well-defined with appropriate version constraints. However, there are two points to consider:
- The commented-out git dependency for pyradiomics suggests a recent switch from a forked version to a specific release.
- The current pyradiomics version "3.0.1a3" is an alpha release, which may not be stable for production use.
Please confirm that the alpha version of pyradiomics (3.0.1a3) is stable enough for your production environment. Consider using a stable release if available.
pyproject.toml (4)
1-16: LGTM: Project configuration updated for Pixi.The transition from Poetry to Pixi in the project configuration looks good. The author information has been updated to include an email address, and the Python version requirement has been appropriately specified.
18-24: LGTM: Pixi-specific configurations added.The new Pixi-specific configurations look good. The channels and platforms are correctly specified, and the package is appropriately listed as an editable dependency for development purposes.
25-39: LGTM: Well-defined environments for different scenarios.The environment configurations for development, publishing, and specific Python versions (3.10, 3.11, 3.12) are well-defined. This setup allows for efficient management of different development and testing scenarios.
86-117: LGTM: Well-configured build and release setup.The build and release configurations are well-defined:
- The build system has been correctly updated to use hatchling.
- Build and release tasks are appropriately configured using hatch and semantic-release.
- The semantic release configuration looks good, allowing for automated versioning and changelog generation.
These changes should streamline the build and release process.
.github/workflows/ci-cd.yml (3)
1-10: LGTM: Improved workflow name and trigger conditions.The changes to the workflow name and trigger conditions are beneficial:
- The capitalized "CI-CD" name improves readability.
- Expanded trigger conditions now cover pushes to all branches and PRs to both main and development, ensuring comprehensive CI/CD coverage.
These changes will help catch issues earlier in the development process.
Line range hint
47-109: LGTM: Well-structured Semantic-Release job.The Semantic-Release job is well-designed:
- Dependency on Unit-Tests ensures releases are based on passing tests.
- Pixi usage aligns with PR objectives for dependency management.
- Steps for printing semantic version and running the release process are clear and logical.
This structure should facilitate consistent and automated versioning and releases.
117-147: Clarify the strategy for PyPI publishing.The Publish-To-PyPi job has been commented out. While this might be part of the transition to Pixi, it's unclear if PyPI publishing is still needed or if it will be implemented differently.
Could you please clarify:
- Is PyPI publishing still part of the release strategy?
- If so, how will it be handled in the new Pixi-based workflow?
- If not, should this commented section be removed entirely?
This clarification will help ensure the CI/CD pipeline meets all necessary release requirements.
🧰 Tools
🪛 yamllint
[error] 144-144: trailing spaces
(trailing-spaces)
[error] 147-147: no new line character at the end of file
(new-line-at-end-of-file)
.github/workflows/ci-cd.yml.disabled (2)
152-153: Check Docker Buildx action versionYou're using
docker/setup-buildx-action@v3, but the latest stable version isv2. Ensure that this version exists or update to the correct version.Confirm the correct action version by checking the action's repository.
125-130: Verify package installation from TestPyPIEnsure that the package name is correct after the transition to Pixi. If the package name has changed, update the installation command accordingly.
Run the following script to check the package name in
pyproject.toml:✅ Verification successful
Package name verification successful
The package name in
pyproject.tomlis correctly set to "readii".🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the package name in pyproject.toml # Test: Extract the package name. Expect: Correct package name displayed. grep -E '^name = ' pyproject.tomlLength of output: 51
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
.gitignoreto exclude additional files and directories.Refactor
pyproject.tomlto transition from Poetry to Pixi, improving modularity and task management.Chores