-
Notifications
You must be signed in to change notification settings - Fork 13
feat: update to python 3.14 #1005
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
Conversation
WalkthroughThe PR bumps Python from 3.12 to 3.14 in CI and project metadata, updates packaging include lists, and refactors Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 |
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: 3
🤖 Fix all issues with AI agents
In @.github/workflows/build_app.yml:
- Line 25: Update the python-version value to include the libc specifier by
changing the string used in the python-version key from
"cpython-3.14-windows-${{ matrix.arch }}" to "cpython-3.14-windows-${{
matrix.arch }}-none" and ensure the matrix.arch entries expand to the
uv-required tokens (use "x86_64" for 64-bit and "x86" for 32-bit) so the
workflow uses the correct platform identifiers.
In `@basilisk/gui/history_msg_text_ctrl.py`:
- Around line 169-171: The partials used to build block renderers in
HistoryMessageTextCtrl are created with functools.partial (which doesn't bind
self) and later invoked without a self argument, causing a TypeError at runtime;
fix by either converting those partials to functools.partialmethod or by passing
self explicitly when calling them (e.g., ensure the call sites that reference
append_* partials pass self as the first argument), and update both the
occurrence around append_suffix/new_block.request.content and the similar
occurrence at the other location flagged (line ~180) so methods like
append_suffix and the other append_* partials are invoked with an explicit self
or are defined with partialmethod.
In `@pyproject.toml`:
- Line 10: The pyproject.toml declares requires-python = ">=3.14,<3.15" but
wxPython has no cp314 wheels yet, so change the Python requirement to a version
that is supported (for example requires-python = ">=3.13,<3.14") in
pyproject.toml to avoid installing on 3.14; update the requires-python entry
and, if you prefer to support 3.14 later, add a TODO comment in pyproject.toml
or your release notes to revisit when wxPython publishes cp314 wheels.
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
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Line 149: The dependency list includes the private module name "_pyrepl",
which is internal and may change; remove "_pyrepl" from the dependency list in
pyproject.toml or make its inclusion conditional/optional (e.g., only add it
with a python_version >= 3.13 marker or as an optional extra) and document why
it's required if you decide to keep it; update references that rely on "_pyrepl"
so they fall back gracefully when the module isn't present.
♻️ Duplicate comments (1)
pyproject.toml (1)
10-10: wxPython cp314 wheel availability remains a blocker.This concern was already raised in a previous review. wxPython currently lacks cp314 wheels on PyPI (only supports up to cp313), which will cause installation failures with the
requires-python = ">=3.14,<3.15"constraint. Consider waiting for wxPython cp314 wheels or targeting Python 3.13 until then.
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
basilisk/gui/history_msg_text_ctrl.py (1)
159-162: Remove explicitselfargument —partialmethodauto-binds it.At line 161,
selfis still being passed explicitly toappend_suffix. Withpartialmethod,selfis automatically bound when called on an instance. This will cause argument misalignment and a runtimeTypeError.Note that line 171 correctly calls
self.append_suffix(new_block_ref, absolute_length)without the explicitself.🐛 Proposed fix
if not self.IsEmpty(): absolute_length = self.append_suffix( - self, new_block_ref, absolute_length + new_block_ref, absolute_length )
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.