feat: setup pixi envs and gha #62
Conversation
📝 WalkthroughWalkthroughThe changes update project configuration and workflows. In the repository’s attributes, a new entry marks Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Checkout as Checkout Step (v4)
participant Pixi as Pixi Installation
participant Ruff as Ruff Check
participant Mypy as Mypy Check
participant QC as Quality Control Summary
GH->>Checkout: Trigger checkout (v4)
Checkout->>Pixi: Install Pixi tool
Pixi->>Ruff: Run Ruff for formatting & linting
Ruff->>Mypy: Execute Mypy for type checking
Mypy->>QC: Gather results
QC-->>GH: Echo success if checks pass
sequenceDiagram
participant GH as GitHub Actions
participant Checkout as Checkout Step (v4)
participant Pixi as Pixi Installation
participant Test as Test Execution
GH->>Checkout: Trigger checkout (v4)
Checkout->>Pixi: Install Pixi tool
Pixi->>Test: Execute tests via Pixi commands
Test-->>GH: Return test results
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 0
🧹 Nitpick comments (3)
.gitignore (1)
162-166: Clarify the lock file strategy for dependency management.
The new entries for Pixi-related files (.pixiand*.egg-info) are correctly added. However, seeing thatpoetry.lockis now explicitly listed (instead of being commented out as before), please confirm that including it in version control is intentional given the switch to Pixi for dependency management..github/workflows/test.yml (2)
16-29: Streamlined quality-control job using Pixi.
The consolidatedquality-controljob now leverages Pixi for dependency setup and running quality checks (formatting, lint, type-check) in a clear and organized manner. The updated checkout action (actions/checkout@v4) and Pixi setup (with specific environment and version parameters) are well configured. One point to note: usingcontinue-on-error: truefor formatting and linting might hide issues if strict enforcement is desired. Please double-check if this behavior is intentional.
60-61: Remove trailing spaces.
Static analysis flagged trailing spaces (e.g., at line 61). Removing these trailing spaces will help maintain conformity with YAML lint rules.- cache: true - + cache: true🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 61-61: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pixi.lockis excluded by!**/*.lockpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (5)
.gitattributes(1 hunks).github/workflows/test.yml(1 hunks).gitignore(1 hunks)snakemake_interface_storage_plugins/storage_object.py(1 hunks)snakemake_interface_storage_plugins/tests.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.py`: Do not try to improve formatting. Do not suggest ...
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
snakemake_interface_storage_plugins/tests.pysnakemake_interface_storage_plugins/storage_object.py
🪛 GitHub Actions: CI
snakemake_interface_storage_plugins/tests.py
[error] 20-20: Skipping analyzing 'snakemake.io': module is installed, but missing library stubs or py.typed marker [import-untyped]
snakemake_interface_storage_plugins/storage_object.py
[error] 12-12: Skipping analyzing 'wrapt': module is installed, but missing library stubs or py.typed marker [import-untyped]
[error] 13-13: Skipping analyzing 'reretry': module is installed, but missing library stubs or py.typed marker [import-untyped]
[error] 16-16: Skipping analyzing 'snakemake_interface_common.exceptions': module is installed, but missing library stubs or py.typed marker [import-untyped]
[error] 17-17: Skipping analyzing 'snakemake_interface_common.logging': module is installed, but missing library stubs or py.typed marker [import-untyped]
[error] 80-80: Incompatible types in assignment (expression has type 'Path', variable has type 'None') [assignment]
[error] 84-84: Incompatible return value type (got 'StorageQueryValidationResult', expected 'bool') [return-value]
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 61-61: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
.gitattributes (1)
1-2: Appropriate configuration for Pixi lock file.
The entry forpixi.lockcorrectly prevents 3-way merges by marking the file as binary and sets YAML syntax highlighting. This aligns well with the repository’s new dependency management approach.snakemake_interface_storage_plugins/tests.py (1)
59-61: Enhanced assertion clarity for storage flags.
The consolidated assertionassert not (self.store_only and self.retrieve_only), ( "store_only and retrieve_only may not be True at the same time" )readably ensures that both flags are not simultaneously set. This concise format improves clarity while retaining the intended functionality.
snakemake_interface_storage_plugins/storage_object.py (1)
95-97: Improved assertion formatting in cache_key.
The change to a multi-line assertion for checking that_overwrite_local_pathisNoneimproves readability without altering the logic. The error message is clear and directly attached to the assertion..github/workflows/test.yml (2)
10-15: Defined minimal permissions for the workflow.
The new permissions block effectively grants only the necessary rights (e.g.,contents: read,checks: write) to the workflow steps. This adheres to the principle of least privilege.
50-64: Updated testing job aligned with Pixi integration.
Thetestingjob is now consistent with the new dependency management approach. The use ofactions/checkout@v4and Pixi for installing dependencies and executing tests (pixi run --environment dev test --show-capture=all -s -vv) is clear and well integrated.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 61-61: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
55-56: Nitpick: Remove Trailing WhitespaceStatic analysis detected trailing spaces on line 56. Please remove any extraneous whitespace to comply with YAML lint guidelines.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 56-56: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/test.yml
[error] 56-56: trailing spaces
(trailing-spaces)
🔇 Additional comments (4)
.github/workflows/test.yml (4)
10-15: Permissions Block Configuration: Validate Principle of Least PrivilegeThe new permissions block specifies read access for contents and write access for checks, issues, and pull-requests. Please verify that these permissions are the minimal ones required for your CI tasks.
16-29: Quality-Control Job Setup: Approved Installation StepsThe restructuring of the quality-control job—with the updated checkout (using actions/checkout@v4) and the "Install Pixi" step (using prefix-dev/[email protected] with proper environment settings and caching enabled)—aligns well with the PR objectives. No issues observed here.
42-44: Quality-Control Summary Step: Clear and SimpleThe "Collect QC" step simply echoes a success message. This approach is clear for a pass scenario. If necessary, consider adding more context or output logging for failed runs if that fits your operational needs.
45-59: Testing Job Verification: Consistent Pixi Setup & Test ExecutionThe testing job now also uses actions/checkout@v4 and installs Pixi with matching configuration (environment
dev, versionv0.42.1, and caching enabled). The test command:pixi run --environment dev test --show-capture=all -s -vvis appropriately set up. Everything here matches the updated quality control approach.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 56-56: trailing spaces
(trailing-spaces)
…ure mypy to ignore missing imports temporarily
…tialization and add type hints
|
Only 2 mypy errors are left: $ pixi run type-check
✨ Pixi task (type-check in dev): mypy snakemake_interface_storage_plugins/
snakemake_interface_storage_plugins/storage_provider.py:214: error: Module "snakemake_interface_storage_plugins.storage_object" has no attribute "StorageObjectReadWrite"; maybe "StorageObjectWrite" or "StorageObjectRead"? [attr-defined]
snakemake_interface_storage_plugins/storage_provider.py:218: error: "StorageProviderBase" has no attribute "storage_object_cls"; maybe "get_storage_object_cls"? [attr-defined]
Found 2 errors in 1 file (checked 9 source files)Looks like these mightve been older features |
| # Class attributes with type hints | ||
| local_prefix: Path | ||
| settings: Optional[StorageProviderSettingsBase] = None | ||
| keep_local: bool = False | ||
| retrieve: bool = True | ||
| is_default: bool = False | ||
| _rate_limiters: Dict[Any, Throttler] = field(default_factory=dict, init=False) | ||
|
|
||
| def __post_init__(self): | ||
| """Hook for subclasses to perform additional initialization. | ||
|
|
||
| Subclasses may override this method to perform additional setup | ||
| after the base class has been initialized and then call the base | ||
| class implementation using `super().__post_init__()`. | ||
| """ | ||
| try: | ||
| local_prefix.mkdir(parents=True, exist_ok=True) | ||
| self.local_prefix.mkdir(parents=True, exist_ok=True) | ||
| except OSError as e: | ||
| raise WorkflowError( | ||
| f"Failed to create local storage prefix {local_prefix}", e | ||
| f"Failed to create local storage prefix {self.local_prefix}", e | ||
| ) | ||
| self.local_prefix = local_prefix | ||
| self.settings = settings | ||
| self.keep_local = keep_local | ||
| self.retrieve = retrieve | ||
| self.is_default = is_default | ||
| self._rate_limiters = dict() | ||
| self.__post_init__() |
There was a problem hiding this comment.
I remember now why I chose to use init instead of a dataclass. This way, plugins that implement this class can start from scratch with post_init and they cannot accidenally overwrite anything that is done in the post_init of this base class.
I guess you should be able to achieve the same type annotations as with a dataclass by adding the types to the attributes that are defined in init.
…lish environment in pyproject.toml
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pixi.lockis excluded by!**/*.lockpyproject.tomlis excluded by!pyproject.toml
📒 Files selected for processing (1)
.github/workflows/release-please.yml(1 hunks)
🔇 Additional comments (5)
.github/workflows/release-please.yml (5)
20-25: Job Renaming and Sequencing Validation.
The renaming of the job topublish-pypinow clearly reflects its purpose. The dependency onrelease-pleasevianeeds: release-pleasecoupled with the conditional check (if: ${{ needs.release-please.outputs.release_created }}) ensures that the publishing job runs only when a release is created. This logic appears solid; just verify that the downstream steps handle the expected artifacts appropriately.
27-28: Checkout Action Upgrade.
Upgrading toactions/checkout@v4is a sensible update. This change should bring improved performance and security features provided by the newer version.
29-33: Setting Up Pixi for Dependency Management.
The new step to install Pixi usingprefix-dev/[email protected]with the parametersenvironments: publishandpixi-version: v0.42.1correctly replaces the old Poetry-based approach. This update aligns with the project’s move toward Pixi for dependency management and release tasks.
35-38: Build and Check Command Update.
The build step now runspixi run --environment publish check-buildto generate source and wheel distributions, along with a build check. This new command should be tested to confirm it produces the expected artifacts in thedist/directory.
40-44: Validate PyPI Publishing Step.
Usingpypa/gh-action-pypi-publish@release/v1for publishing to PyPI is correct. Confirm that the secrets management forPYPI_PASSWORD(using${{ secrets.PYPI_TOKEN }}) is appropriately configured to secure your credentials.
github-project-automation
bot
moved this from In review
to Done
in Snakemake Hackathon March 2025
2 participants
Summary by CodeRabbit