-
-
Notifications
You must be signed in to change notification settings - Fork 2
Chore | Add support for python 3.13 #310
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
base: master
Are you sure you want to change the base?
Conversation
Warning Rate limit exceeded@angelofenoglio has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 27 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughAdded Python 3.13 support across the project by updating GitHub Actions workflows (release, test, integration, unit), expanding test matrices, and updating README and pyproject.toml to declare Python 3.13 compatibility. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Repo as Repository
participant GH as GitHub Actions
participant Tests as Test Matrix
participant Docs as README/pyproject
Dev->>Repo: Push CI/docs metadata changes
Repo->>GH: Trigger workflows (release, unit, integration)
GH->>Tests: Run jobs across Python versions (3.9,3.10,3.11,3.12,3.13)
Dev->>Docs: Update README and pyproject.toml to include 3.13
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
8480c90
to
dc32855
Compare
Pull Request Test Coverage Report for Build 16888289585Details
💛 - Coveralls |
There was a problem hiding this 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 comments (1)
README.md (1)
131-133
: Wrong virtual-env name breaks the setup instructionsThe snippet still refers to the old
leverage_py_39_venv
, which no longer exists in the steps above.-pyenv local leverage_py_39_venv +pyenv local leverage_py_313_venv # or leverage_py_312_venv if you keep the 3.12 example
♻️ Duplicate comments (1)
README.md (1)
115-116
: Verify patch-level availability before publishing
pyenv install 3.13.3
will break newcomers if 3.13.3 hasn’t been released yet (final 3.13 might ship as 3.13.0). Consider using a placeholder (3.13.x
) or a released tag.
🧹 Nitpick comments (5)
.github/workflows/tests-unit.yml (1)
10-10
: Matrix expanded 👍 – but coverage upload still tied to 3.9.Adding 3.13 is great. However, the Coveralls step later on is gated by
if: matrix.python-version == '3.9'
. This means coverage will not be
reported for the brand-new interpreter.Two quick options:
- Switch the condition to
'3.13'
so the latest version drives the badge.- Remove the
if
and let only the first job (viamatrix.include
orneeds
) upload once.Otherwise the badge may hide interpreter-specific gaps.
pyproject.toml (2)
20-22
: Classifier order/value looks fine – minor nit: keep them sorted to avoid merge churn:"Programming Language :: Python :: 3.9", "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13",
30-30
: Simplify the Python version constraint for maintainability.Enumerating every minor line means another commit each time Python ships.
Poetry understands range syntax, so the same intent can be expressed as:-python = "~3.9 || ~3.10 || ~3.11 || ~3.12 || ~3.13" +python = ">=3.9,<4.0"This still excludes 3.8 and earlier but automatically admits 3.14+ when you’re ready.
.github/workflows/tests-integration.yaml (1)
28-28
: Avoid pinning patch versions in the test matrix.
actions/setup-python
resolves3.13
to the latest available patch.
Hard-coding3.13.3
risks CI breakage the moment a patch disappears or the cache misses.- python-version: ['3.9.15', '3.10.8', '3.11.8', '3.12.7', '3.13.3'] + python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']This keeps the matrix current with zero maintenance.
README.md (1)
120-124
: Example still pins to 3.12 – switch to 3.13 or make version-agnosticTo be consistent with the new 3.13 support you just added, either:
- Update the heading and command to use 3.13.x, or
- Replace the concrete version with a placeholder to avoid future churn.
-Create a virtual environment for your project using Python 3.12.7: +Create a virtual environment for your project using Python 3.13.x: @@ -pyenv virtualenv 3.12.7 leverage_py_312_venv +pyenv virtualenv 3.13.x leverage_py_313_venv
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lock
is excluded by!**/*.lock
📒 Files selected for processing (6)
.github/workflows/release-pypi-build-push-package.yml
(1 hunks).github/workflows/release-pypi-build-push-test-package.yml
(1 hunks).github/workflows/tests-integration.yaml
(1 hunks).github/workflows/tests-unit.yml
(1 hunks)README.md
(3 hunks)pyproject.toml
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: the toolbox image tags `1.3.5-0.2.0`, `1.5.0-0.2.0`, and `1.6.0-0.2.0` exist in the registry....
Learnt from: exequielrafaela
PR: binbashar/leverage#289
File: .github/workflows/tests-integration.yaml:28-29
Timestamp: 2024-10-17T16:31:27.570Z
Learning: The toolbox image tags `1.3.5-0.2.0`, `1.5.0-0.2.0`, and `1.6.0-0.2.0` exist in the registry.
Applied to files:
.github/workflows/tests-integration.yaml
🔇 Additional comments (1)
README.md (1)
73-76
: Supported-version list update LGTMAdding “Python 3.13.x” keeps the docs aligned with the new CI matrix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
.github/workflows/release-pypi-build-push-package.yml (2)
26-31
: Wrong env var export and incorrect Poetry PATH
export ENV POETRY_VIRTUALENVS_CREATE=false
sets a variable named ENV; it doesn’t set POETRY_VIRTUALENVS_CREATE.- Poetry installed via the official script is typically at
$HOME/.local/bin
, not~/.poetry/bin
.- name: Install dependencies using Poetry run: | echo "[INFO] Installing dependencies..." - export ENV POETRY_VIRTUALENVS_CREATE=false - export PATH="${PATH}:${HOME}/.poetry/bin" + export POETRY_VIRTUALENVS_CREATE=false + export PATH="$HOME/.local/bin:$PATH" poetry install --with=dev
19-25
: Ensure Poetry is on PATH and supports Python 3.13Both Poetry 1.8.2 and 1.8.3 lack a “Programming Language :: Python :: 3.13” classifier, so pinning to 1.8.2 (or 1.8.3) will break under Python 3.13. You should also prepend
$HOME/.local/bin
to yourPATH
before invokingpoetry
.• Add
export PATH="$HOME/.local/bin:$PATH"
so the installer’spoetry
binary is found.
• Either remove thePOETRY_VERSION
pin (so the installer grabs the latest release that includes Python 3.13 support)
• Or explicitly pin to the first Poetry release that adds Python 3.13 in its classifiers (e.g. ≥1.9.x once confirmed).Proposed diff:
- name: Install Poetry run: | echo "[INFO] Installing Poetry..." + # Make sure the installer’s poetry binary is on PATH + export PATH="$HOME/.local/bin:$PATH" - curl -sSL https://install.python-poetry.org | POETRY_VERSION=1.8.2 python - + # Install latest Poetry (removes pin so Python 3.13 support is included) + curl -sSL https://install.python-poetry.org | python - echo "[INFO] Configuring Poetry..." poetry config virtualenvs.create false # Prevent poetry from creating a virtual environment
♻️ Duplicate comments (2)
.github/workflows/release-pypi-build-push-package.yml (2)
11-11
: actions/checkout@v4 upgrade: LGTMThis aligns with modern runners and prior feedback.
15-17
: actions/setup-python@v5 with Python 3.13: LGTMMatches declared 3.13 support and fixes the actionlint complaint.
🧹 Nitpick comments (1)
.github/workflows/release-pypi-build-push-package.yml (1)
15-17
: Speed up installs with built-in Poetry cacheactions/setup-python@v5 supports caching. Recommend enabling it for Poetry.
- uses: actions/setup-python@v5 with: - python-version: '3.13' + python-version: '3.13' + cache: 'poetry'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/release-pypi-build-push-package.yml
(1 hunks).github/workflows/release-pypi-build-push-test-package.yml
(1 hunks)README.md
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- .github/workflows/release-pypi-build-push-test-package.yml
🔇 Additional comments (1)
.github/workflows/release-pypi-build-push-package.yml (1)
33-36
: Version bump during a published release can desync tag and packageRunning
make bump-version-ci
in a workflow triggered by release: published may produce a commit that no longer matches the released tag. Verify your intended flow (tag -> build -> publish) vs. (bump -> tag -> release).If the canonical flow is “tag first, then publish,” consider:
- Removing the bump from this job, or
- Triggering this pipeline earlier (pre-release) and using the tag created by the bump step.
with: | ||
ref: master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Release builds should check out the tag, not master
On release: published, building from master can package the wrong commit. Use the event ref (the tag) or omit ref to default to the triggering ref.
- with:
- ref: master
+ with:
+ # Build the exact commit associated with the published release tag
+ ref: ${{ github.ref }}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
with: | |
ref: master | |
with: | |
# Build the exact commit associated with the published release tag | |
ref: ${{ github.ref }} |
🤖 Prompt for AI Agents
.github/workflows/release-pypi-build-push-package.yml lines 12-13: the workflow
currently forces checkout of master which can build the wrong commit for
tag-triggered releases; update the checkout step to use the triggering ref or
omit ref so actions/checkout defaults to it — replace or change the ref value to
use the workflow expression (for example ref: ${{ github.ref }}) or remove the
ref line entirely so the tag that triggered the workflow is checked out.
9fb2ac5
to
0049202
Compare
What?
Summary by CodeRabbit
Documentation
Chores