Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces Poetry-based tooling with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/src/contributing.md (2)
90-112:⚠️ Potential issue | 🟡 MinorUpdate versioning documentation to reflect current tooling.
The documentation references
poetry-dynamic-versioningbut the project configuration (pyproject.toml) explicitly requiressetuptools-scm>=8anduv-dynamic-versioning. Update lines 92, 95, and 111 to reflect the actual versioning tools used to avoid misleading contributors.Suggested changes
-We use [`poetry-dynamic-versioning`](https://github.com/mtkennerly/poetry-dynamic-versioning) to help version this software through [`PEP 440`](https://peps.python.org/pep-0440/) standards. +We use `setuptools-scm` with `uv-dynamic-versioning` to produce `PEP 440` versions. @@ -`poetry-dynamic-versioning` uses `git` tags to help distinguish version data. +Versioning uses `git` tags to help distinguish version data. @@ -CytoDataFrame version specifications are controlled through [`poetry-dynamic-versioning`](https://github.com/mtkennerly/poetry-dynamic-versioning) which leverages [`dunamai`](https://github.com/mtkennerly/dunamai) +CytoDataFrame version specifications are controlled through `setuptools-scm` and `uv-dynamic-versioning`
18-28:⚠️ Potential issue | 🟡 Minor
uv sync --all-groupscommand is correct.Confirmed per current uv documentation: this is the recommended approach for local development/contributors who want all tooling (lint, test, docs, etc.) installed without listing each group individually.
However, the versioning section (lines 90-96) still references
poetry-dynamic-versioningand should be updated to align with the migration touv. Review whether versioning tooling has been migrated or if this reference is now stale.
🤖 Fix all issues with AI agents
In @.github/dependabot.yml:
- Around line 15-16: Update the misleading comment text that says "every week"
to match the configured interval "monthly" for the Dependabot schedule entries;
locate the comment lines adjacent to the interval: "monthly" (the two
occurrences where the comment currently reads "Check for updates to GitHub
Actions every week") and change the wording to reflect "monthly" so the comment
matches interval: "monthly".
In @.github/workflows/dependabot.yml:
- Around line 31-34: Update the workflow to pin the astral-sh/setup-uv and
j178/prek-action actions to immutable commit SHAs and provide an explicit uv
version via the setup-uv `version` input: replace uses: astral-sh/setup-uv@v7
with uses: astral-sh/setup-uv@<commit-sha> and add a with: version: "0.10.0" (or
project-configured version), and replace uses: j178/prek-action@v1 with uses:
j178/prek-action@<commit-sha> so both actions are tied to full commit SHAs to
prevent CI drift and supply-chain risk.
In @.pre-commit-config.yaml:
- Line 18: The pre-commit hook for pyproject-fmt is pinned to a non-existent
release "v2.16.0"; update the rev value in .pre-commit-config.yaml (the
pyproject-fmt hook's rev) to a real PyPI release such as "v2.12.1" (or the
latest available), then run pre-commit autoupdate / pre-commit install to verify
hooks install correctly; retain the existing hook args and uv.lock usage as-is.
In `@pyproject.toml`:
- Around line 1-3: The pyproject build configuration is incompatible because
uv-dynamic-versioning is a Hatchling-only plugin but build-backend is set to
setuptools.build_meta; either remove "uv-dynamic-versioning" from the requires
list and keep build-backend = "setuptools.build_meta" (relying on
"setuptools-scm" only), or change build-backend to "hatchling.build" and keep
"uv-dynamic-versioning" in requires; update the [build-system] section
accordingly referencing build-backend, requires, and the uv-dynamic-versioning
and setuptools-scm symbols.
In `@tests/data/cytotable/NF1_cellpainting_data_shrunken/create_image_data.py`:
- Around line 5-7: The docstring command references the wrong filename: update
the usage example so it matches the actual script name (create_image_data.py) or
rename the file to create.py to match the docstring; specifically, edit the
top-level comment in create_image_data.py to either change the example command
to `uv run python
tests/data/cytotable/NF1_cellpainting_data_shrunken/create_image_data.py` or
rename the file and update any imports/CI references so they point to
`create.py`.
🧹 Nitpick comments (2)
pyproject.toml (2)
22-43: Remove redundant<3.9dependency markers.With
requires-python >=3.11, thepython_full_version<'3.9'branches are unreachable. Consolidating these reduces confusion and lock churn.Suggested cleanup
- "pandas<2.2.2; python_full_version<'3.9'", - "pandas>=2.2.2,<3; python_full_version>='3.9'", + "pandas>=2.2.2,<3", @@ - "pywavelets>=1.4.1,<2; python_full_version<'3.9'", - "pywavelets>1.4.1; python_full_version>='3.9'", + "pywavelets>1.4.1", @@ - "scikit-image>=0.19.3,<0.20; python_full_version<'3.9'", - "scikit-image>0.19.3; python_full_version>='3.9'", + "scikit-image>0.19.3",
5-21: Align Ruff target-version with the project's Python floor.
requires-pythonis>=3.11, buttool.ruff.target-versionis set topy38(line 87). Sincetarget-versiontakes precedence overrequires-python, Ruff will only enforce compatibility checks for Python 3.8+, not 3.11+. Update it topy311to match the actual minimum supported version.Suggested update
[tool.ruff] -target-version = "py38" +target-version = "py311"
tests/data/cytotable/NF1_cellpainting_data_shrunken/create_image_data.py
Outdated
Show resolved
Hide resolved
jenna-tomkinson
left a comment
jenna-tomkinson
left a comment
There was a problem hiding this comment.
LGTM! I had two general questions but seems like a super easy port over.
Co-Authored-By: Jenna Tomkinson <[email protected]>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pyproject.toml (1)
32-39: Remove unreachable Python-version dependency branches.Lines 32–39 contain conditional branches for
python_full_version<'3.9', but Line 14 restricts the project to Python>=3.11. These branches are unreachable and should be simplified by keeping only the>=3.9variants.♻️ Suggested simplification
- "pandas<2.2.2; python_full_version<'3.9'", - "pandas>=2.2.2,<3; python_full_version>='3.9'", + "pandas>=2.2.2,<3", ... - "pywavelets>=1.4.1,<2; python_full_version<'3.9'", - "pywavelets>1.4.1; python_full_version>='3.9'", + "pywavelets>1.4.1", ... - "scikit-image>=0.19.3,<0.20; python_full_version<'3.9'", - "scikit-image>0.19.3; python_full_version>='3.9'", + "scikit-image>0.19.3",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 32 - 39, Remove the unreachable python_full_version<'3.9' conditional branches and keep only the >=3.9 variants: replace lines like "pandas<2.2.2; python_full_version<'3.9'", "pywavelets>=1.4.1,<2; python_full_version<'3.9'", and "scikit-image>=0.19.3,<0.20; python_full_version<'3.9'" with their corresponding >=3.9 forms (the lines already present: "pandas>=2.2.2,<3; python_full_version>='3.9'", "pywavelets>1.4.1; python_full_version>='3.9'", "scikit-image>0.19.3; python_full_version>='3.9'"), removing the conditional suffixes entirely if you prefer a single unconditional requirement list since the project requires Python >=3.11.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pyproject.toml`:
- Around line 32-39: Remove the unreachable python_full_version<'3.9'
conditional branches and keep only the >=3.9 variants: replace lines like
"pandas<2.2.2; python_full_version<'3.9'", "pywavelets>=1.4.1,<2;
python_full_version<'3.9'", and "scikit-image>=0.19.3,<0.20;
python_full_version<'3.9'" with their corresponding >=3.9 forms (the lines
already present: "pandas>=2.2.2,<3; python_full_version>='3.9'",
"pywavelets>1.4.1; python_full_version>='3.9'", "scikit-image>0.19.3;
python_full_version>='3.9'"), removing the conditional suffixes entirely if you
prefer a single unconditional requirement list since the project requires Python
>=3.11.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pyproject.toml`:
- Line 14: The pyproject.toml sets requires-python = ">=3.11,<3.14" so any
dependency entries guarded with the marker "python_full_version<'3.9'" (and
their paired ">=3.9" variants) are unreachable; remove those conditional
dependency branches and collapse each pair into a single unconditional
dependency entry (i.e., delete the "python_full_version<'3.9'" markers and the
redundant >=3.9 alternatives), leaving only the single dependency specification
for each affected package so the TOML contains no dead conditional branches.
- Line 3: Update the setuptools minimum in the pyproject.toml requires array
from "setuptools>=64" to "setuptools>=77.0.0" so the SPDX-style project.license
= "BSD-3-Clause" (PEP 639) is supported; locate the requires = [
"setuptools>=64", "setuptools-scm>=8" ] entry and bump the setuptools version
constraint to "setuptools>=77.0.0".
|
Thank you @jenna-tomkinson for the review! Merging this in now. |
2 participants
Description
This PR migrates from
poetrytouvfor environment management in the project.What kind of change(s) are included?
Checklist
Please ensure that all boxes are checked before indicating that this pull request is ready for review.
Summary by CodeRabbit
Chores
Documentation