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

feat: transfer project management to Pixi #133

Merged
merged 16 commits into from
Nov 15, 2024
Merged

feat: transfer project management to Pixi #133

merged 16 commits into from
Nov 15, 2024

Conversation

jjjermiah
Copy link
Contributor

@jjjermiah jjjermiah commented Nov 15, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new configuration file for project settings and dependencies.
    • Added versioning to the module with __version__ set to "1.6.0-rc.2".
  • Changes

    • Updated workflow configurations for testing and code coverage.
    • Restructured ignored files in the ignore list.
    • Transitioned project configuration from Poetry to Hatch in the project configuration file.
  • Bug Fixes

    • Removed obsolete files to streamline project management.

@jjjermiah jjjermiah requested a review from skim2257 November 15, 2024 16:12
Copy link
Contributor

coderabbitai bot commented Nov 15, 2024

Walkthrough

The changes encompass updates to various configuration files and workflows for the project "med-imagetools." Key modifications include the introduction of a new .gitattributes entry for the pixi.lock file, significant alterations to the GitHub Actions workflow in .github/workflows/main.yml, and the addition of a new pixi.toml configuration file. The project has transitioned from a Poetry-based setup to a Hatch-based configuration in pyproject.toml, along with the removal of several files such as requirements.txt, ruff.toml, and setup.py. New sections for coverage and linting have also been added in their respective configuration files.

Changes

File Path Change Summary
.gitattributes Added entry to treat pixi.lock as a YAML file and mark it as generated.
.github/workflows/main.yml Renamed workflow to "Test," updated triggers, removed lint job, added Unit-Tests and Code-Coverage jobs, and modified job configurations.
.gitignore Added entries for ignoring pixi files and example directories, removed demo.py and README_backup.md from ignore list.
config/coverage.toml Added new sections [tool.coverage.run] and [tool.coverage.report].
config/ruff.toml Introduced new [lint] section with ignored error codes and a commented-out select directive.
pixi.toml New configuration file detailing project settings, dependencies, and environments.
pyproject.toml Transitioned from Poetry to Hatch, updated project metadata, dependencies, and build system configuration.
requirements.txt Removed file listing Python package dependencies.
ruff.toml Removed linter configuration file.
setup.py Removed package setup configuration file.
src/imgtools/__init__.py Added __version__ variable with value "1.6.0-rc.2".

Poem

🐰 In the garden of code, we hop and play,
With changes sprouting, brightening the day.
New files and workflows, all set to bloom,
A version so fresh, it chases the gloom.
So here’s to the updates, let’s cheer and rejoice,
For every new feature, we celebrate with voice! 🎉


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.

Copy link
Contributor

@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: 6

🧹 Outside diff range and nitpick comments (9)
pyproject.toml (2)

21-36: Consider relaxing some version constraints

While the major version pins are good for stability, some minor version constraints might be too restrictive. For example:

  • pandas>=2.2.2,<3 - Consider allowing earlier 2.x versions if compatible
  • scikit-image>=0.23.2,<1 - The upper bound <1 might be too restrictive as the package is approaching 1.0

54-57: Clarify the relationship between CLI commands

The presence of both autopipeline and betapipeline might confuse users. Consider:

  1. Adding deprecation warnings in the original version
  2. Documenting the differences and migration path
  3. Adding version tags to indicate stability
pixi.toml (4)

4-4: Update placeholder description

The current description appears to be a placeholder. Please replace it with a meaningful description of the project's purpose and functionality.


45-58: Consider enabling parallel testing

The --numprocesses=auto option is commented out. If there's no specific reason to disable parallel testing, enabling it could significantly improve test execution time.

  "pytest",
- # "--numprocesses=auto", 
+ "--numprocesses=auto",
  "-s",

51-53: Consider simplifying coverage configuration path

The coverage configuration and report paths could be simplified by using a consistent directory structure:

- "--cov-report=xml:coverage-report/coverage.xml",
- "--cov-config=config/coverage.toml",
+ "--cov-report=xml:.coverage/coverage.xml",
+ "--cov-config=.coverage/config.toml",

90-90: Improve semantic version command

The current command uses echo with a subshell, which could be simplified. Also, consider adding error handling.

- semver = 'echo "Next Version is: $(semantic-release version --print)"'
+ semver = "semantic-release version --print"
.github/workflows/main.yml (3)

1-8: Consider optimizing workflow triggers

Running the workflow on all branches ("*") might consume unnecessary CI resources. Consider limiting the triggers to:

  • Main/development branches for pushes
  • Pull request branches that could potentially be merged
on:
  push:
-   branches: ["*"]
+   branches: ["main", "devel"]
  pull_request:
-   branches: ["*"]
+   branches: ["main", "devel"]

Line range hint 140-144: Update PyPI authentication configuration

The current PyPI authentication setup uses both user and password. The recommended approach is to use just the token.

  uses: pypa/gh-action-pypi-publish@release/v1
  with:
    verbose: true
-   user: ${{ secrets.pypi_token }}
-   password: ${{ secrets.pypi_api }}
+   password: ${{ secrets.pypi_token }}

147-167: Remove commented out job configuration

If the does-the-pip-squeak job is no longer needed, it should be removed rather than left as commented code. This helps maintain a cleaner configuration file.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 84aa480 and 35f0c8f.

⛔ Files ignored due to path filters (2)
  • pixi.lock is excluded by !**/*.lock
  • poetry.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • .gitattributes (1 hunks)
  • .github/workflows/main.yml (5 hunks)
  • .gitignore (1 hunks)
  • config/coverage.toml (1 hunks)
  • config/ruff.toml (1 hunks)
  • pixi.toml (1 hunks)
  • pyproject.toml (1 hunks)
  • requirements.txt (0 hunks)
  • ruff.toml (0 hunks)
  • setup.py (0 hunks)
  • src/imgtools/__init__.py (1 hunks)
💤 Files with no reviewable changes (3)
  • requirements.txt
  • ruff.toml
  • setup.py
✅ Files skipped from review due to trivial changes (4)
  • .gitattributes
  • config/coverage.toml
  • config/ruff.toml
  • src/imgtools/init.py
🧰 Additional context used
🪛 actionlint
.github/workflows/main.yml

102-102: the runner of "actions/setup-python@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🔇 Additional comments (9)
pyproject.toml (3)

59-69: LGTM! Build system configuration looks good

The Hatch build configuration is well-structured with appropriate include patterns and explicit build directory setting.


71-86: LGTM! Semantic release configuration is well-structured

The configuration properly handles versioning for both main and development branches, with appropriate prerelease token handling.


1-18: Verify the license change implications

The change from GPL-3.0 to MIT license is a significant modification that requires careful consideration. This change makes the software more permissive but could have implications for existing users and contributors.

✅ Verification successful

License change from GPL-3.0 to MIT has been properly implemented

The license change has been properly documented and implemented across the codebase:

  • LICENSE file contains the complete MIT license text
  • CHANGELOG.md documents the license update in PR fix: License updated to MIT #119
  • README.md correctly references the MIT license
  • No remaining GPL references found in the codebase
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for GPL-3.0 license references and verify license files

# Check for LICENSE file changes
git diff HEAD~1 LICENSE

# Look for any remaining GPL references
rg -i "GPL|GNU" --type txt --type md

# Check if NOTICE or COPYING files exist (common for GPL)
fd -g "NOTICE*" -g "COPYING*"

Length of output: 192


Script:

#!/bin/bash
# Let's try a different approach to verify license-related changes

# Check if LICENSE file exists and its content
if [ -f LICENSE ]; then
    echo "=== Current LICENSE content ==="
    cat LICENSE
fi

# Check commit history for license-related changes
git log --grep="license\|GPL\|MIT" --pretty=format:"%h %s" -i

# Search for any license declarations in documentation
rg -i "licen[cs]e" --type md

Length of output: 1632

pixi.toml (3)

8-9: LGTM: Appropriate development setup

The editable installation of the package is correctly configured for development purposes.


11-20: LGTM: Well-structured environment configuration

The environment setup supports multiple Python versions and has a comprehensive default environment with all necessary features.

#!/bin/bash
# Verify Python version compatibility in the codebase
# Check for version-specific imports or syntax
rg -l "(?i)(python_version|sys\.version_info|typing\.TYPE_CHECKING|__future__)" src/

# Check for potential compatibility issues in dependencies
if [ -f "pyproject.toml" ]; then
    grep -A 5 "python_requires" pyproject.toml
fi

6-6: Consider adding Windows platform support

The platforms list currently only includes Linux and macOS (ARM). Consider adding Windows support (win-64) if the project is intended to be cross-platform.

.gitignore (1)

219-219: LGTM!

The blank line separator improves readability by grouping related ignore patterns.

.github/workflows/main.yml (2)

55-60: Verify coverage artifact paths

Ensure the coverage report paths are consistent between upload in Unit-Tests job and download in Code-Coverage job.

✅ Verification successful

Let me check the workflow file to verify the artifact paths between jobs.


Let me check the complete artifact configuration in both jobs.


Artifact paths are consistent between jobs

The coverage artifact configuration is correctly set up:

  • Upload job: Uses name coverage-report without specific path (defaults to repository root)
  • Download job: Uses name coverage-report with path coverage-report
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for coverage path configurations
if [ -f "pyproject.toml" ]; then
    echo "Coverage paths in pyproject.toml:"
    cat pyproject.toml | grep -A 5 "\[tool.coverage"
fi

if [ -f "config/coverage.toml" ]; then
    echo "Coverage paths in coverage.toml:"
    cat config/coverage.toml
fi

Length of output: 347


Script:

#!/bin/bash
# Check the workflow file for coverage artifact paths
echo "Checking coverage artifact paths in workflow:"
rg "coverage.*artifact" .github/workflows/main.yml -B2 -A2

Length of output: 567


Script:

#!/bin/bash
# Get the complete artifact configuration context
echo "Checking complete artifact configurations in workflow:"
rg "Upload coverage|Download coverage" .github/workflows/main.yml -B2 -A5

Length of output: 752


19-20: Verify Windows compatibility status

Windows support is currently commented out. This might limit the package's cross-platform compatibility.

✅ Verification successful

Windows support is intentionally disabled with known compatibility considerations

The codebase shows evidence of Windows compatibility being a known consideration:

  • Windows support was previously included in CI but intentionally removed (commented out with "removed for now" note)
  • The codebase includes Windows-specific handling:
    • Path character sanitization for Windows in src/imgtools/io/writers.py
    • Windows-specific dependency (colorama) in pixi.lock
  • No Windows-specific test skips or platform-specific code blocks that would indicate incompatibility

The temporary removal from CI suggests this is a known state rather than an oversight, likely pending specific Windows-related improvements before re-enabling.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there are any OS-specific code paths or dependencies

# Search for OS-specific imports or conditions
rg -l 'import (os|sys|platform)' || rg -l 'sys\.platform|os\.name'

# Check pixi.toml for OS-specific dependencies
if [ -f "pixi.toml" ]; then
    cat pixi.toml | grep -i "platform.*=.*\"windows\""
fi

Length of output: 466


Script:

#!/bin/bash
# Let's check for Windows-specific code paths or potential compatibility issues

# Search for Windows-specific path handling or system calls
rg -i 'win(32|64|dows)' -A 2 -B 2

# Check for path separators that might cause issues on Windows
rg -i '(\\\\|//)' --type py

# Look for any GitHub issues or discussions about Windows support
rg -i 'windows' .github/ || true

# Check test files for any Windows-specific test skips or conditions
rg -i '(skipif|pytest\.mark).*(windows|platform)' tests/

Length of output: 2926

pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
Copy link

codecov bot commented Nov 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 45.75%. Comparing base (e7cca2a) to head (4c72576).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #133      +/-   ##
==========================================
+ Coverage   45.73%   45.75%   +0.01%     
==========================================
  Files          30       30              
  Lines        2871     2872       +1     
==========================================
+ Hits         1313     1314       +1     
  Misses       1558     1558              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@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: 3

🧹 Outside diff range and nitpick comments (8)
pyproject.toml (3)

18-18: Consider narrowing Python version range

The current Python requirement >=3.10,<4 is very broad. Consider setting an upper bound to a specific minor version based on your testing matrix (e.g., >=3.10,<3.13).


35-35: Add upper bound for attrs dependency

All other dependencies have upper version bounds except attrs. Consider adding an upper bound to prevent potential breaking changes:

-  "attrs>=23.2.0",
+  "attrs>=23.2.0,<24",

54-56: Consider consolidating autopipeline implementations

The presence of both autopipeline and betapipeline (refactored) suggests an incomplete migration. Consider:

  1. Documenting the differences between these implementations
  2. Creating a migration plan to consolidate into a single implementation
  3. Marking the legacy implementation as deprecated if needed
.github/workflows/main.yml (5)

1-8: Optimize workflow triggers

The current configuration runs the full workflow on any branch push or PR, which could waste CI resources. Consider:

  • Limiting pushes to main/development branches
  • Using path filters to run only when relevant files change
name: Test

on:
  push:
-   branches: ["*"]
+   branches: ["main", "devel"]
+   paths:
+     - "**/*.py"
+     - "pyproject.toml"
+     - "pixi.toml"
  pull_request:
-   branches: ["*"]
+   branches: ["main", "devel"]
+   paths:
+     - "**/*.py"
+     - "pyproject.toml"
+     - "pixi.toml"

61-68: Enhance Codecov upload reliability

Consider adding these improvements to the Codecov step:

  • Retry mechanism for flaky uploads
  • Validation step to ensure coverage file exists
+ - name: Verify coverage file exists
+   run: |
+     if [ ! -f ./coverage-report/coverage.xml ]; then
+       echo "Coverage file not found!"
+       exit 1
+     fi

  - name: Use Codecov to track coverage
    uses: codecov/codecov-action@v3
    with:
      token: ${{ secrets.CODECOV_TOKEN }}
      files: ./coverage-report/coverage.xml
      fail_ci_if_error: true
+     retry_on_error: true
+     retry_count: 3
      verbose: true
      name: codecov-umbrella

Line range hint 73-80: Review permission scopes

The job requests broad permissions. Consider limiting them to only what's needed:

  • Remove actions: read as it's not used
  • Consider if issues and pull-requests write access is necessary
permissions:
  id-token: write
- actions: read
  contents: write
  packages: write
- issues: write
- pull-requests: write

Line range hint 140-144: Security: Streamline PyPI authentication

The current configuration uses both token and password which is redundant and less secure.

  - name: Publish to PyPI
    uses: pypa/gh-action-pypi-publish@release/v1
    with:
      verbose: true
-     user: ${{ secrets.pypi_token }}
-     password: ${{ secrets.pypi_api }}
+     password: ${{ secrets.PYPI_TOKEN }}

147-167: Remove or document commented code

The does-the-pip-squeak job is commented out without explanation. Either:

  • Remove it if it's no longer needed
  • Document why it's disabled and when it will be re-enabled
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 35f0c8f and 4c72576.

⛔ Files ignored due to path filters (1)
  • pixi.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • .github/workflows/main.yml (5 hunks)
  • .gitignore (1 hunks)
  • pixi.toml (1 hunks)
  • pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .gitignore
  • pixi.toml
🔇 Additional comments (2)
pyproject.toml (2)

70-85: LGTM! Well-structured semantic release configuration

The semantic release configuration is well-defined with:

  • Clear version variable mappings
  • Appropriate branch patterns for main and development
  • Proper prerelease token configuration

16-16: Verify license change from GPL-3.0 to MIT

The license has been changed from GPL-3.0 to MIT. This is a significant change that requires careful consideration.

✅ Verification successful

License change from GPL-3.0 to MIT is properly implemented

The LICENSE file has been updated with the standard MIT license text, and no references to GPL/GNU were found in the codebase. The change is consistently reflected in both pyproject.toml and LICENSE file.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify license change implications

# Check if LICENSE file exists and contains MIT license text
if [ -f LICENSE ]; then
  echo "LICENSE file content:"
  cat LICENSE
fi

# Check for any GPL-3.0 references in the codebase
echo "Checking for GPL-3.0 references:"
rg -i "GPL|GNU" --type txt --type md

Length of output: 1279

.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
.github/workflows/main.yml Show resolved Hide resolved
Copy link
Collaborator

@skim2257 skim2257 left a comment

Choose a reason for hiding this comment

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

the future of med-imagetools is bright!

@skim2257 skim2257 merged commit a268211 into main Nov 15, 2024
27 checks passed
@jjjermiah jjjermiah deleted the devel branch November 18, 2024 14:47
jjjermiah added a commit that referenced this pull request Nov 18, 2024
* feat: started port to pixi

* feat: updated test suite for pixi and started refactor of cicd

* build: added coverage to gha

* fix: explicitly direct location of package code and update pixi in pipeline

* fix: added linux to pixi

* chore: update GitHub Actions workflow for improved deployment process

* chore: clean up GitHub Actions workflow by removing commented-out jobs and updating dependencies

* chore: update GitHub Actions workflow to deploy from both main and devel branches, and refresh dependencies in pixi.lock

* chore(sem-ver): 1.6.0-rc.1

* fix: update version_toml path in pyproject.toml for semantic release

* chore(sem-ver): 1.6.0-rc.2

* chore: remove requirements.txt file and poetry.lock

* chore: update authors and maintainers in pyproject.toml,  and add __version__ in __init__.py

* chore: remove setup.py file as part of project restructuring

* chore: update pixi.lock and add ruff configuration for linting

* chore: fix some recommendations from coderabbit

---------

Co-authored-by: semantic-release <semantic-release>
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.

2 participants