-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add support for Python 3.13 #6132
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
|
Thank you for the PR! The changelog has not been updated, so here is a friendly reminder to check if you need to add an entry. |
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.
Pull Request Overview
This PR adds support for Python 3.13 and 3.14 by addressing compatibility issues with dependencies and platform-specific code. The changes ensure the codebase can run on newer Python versions while maintaining backward compatibility.
Key changes:
- Updated
librosadependency to version>=0.11to fix numpy type compatibility issues in Python 3.13+ - Replaced low-level
fcntl.ioctlterminal width detection with cross-platformshutil.get_terminal_size()to handle Python 3.14's stricter type requirements - Extended CI test matrix to include Python 3.13 and 3.14
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pyproject.toml | Updated librosa dependency constraint from ^0.10.2.post1 to >=0.11 for Python 3.13+ compatibility |
| beets/ui/init.py | Refactored term_width() to use shutil.get_terminal_size() instead of platform-specific fcntl.ioctl, added type hint and caching decorator |
| .github/workflows/ci.yaml | Added Python 3.13 and 3.14 to the CI test matrix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `beets/ui/__init__.py:706` </location>
<code_context>
return columns if columns else config["ui"]["terminal_width"].get(int)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if-expression with `or` ([`or-if-exp-identity`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/or-if-exp-identity))
```suggestion
return columns or config["ui"]["terminal_width"].get(int)
```
<br/><details><summary>Explanation</summary>Here we find ourselves setting a value if it evaluates to `True`, and otherwise
using a default.
The 'After' case is a bit easier to read and avoids the duplication of
`input_currency`.
It works because the left-hand side is evaluated first. If it evaluates to
true then `currency` will be set to this and the right-hand side will not be
evaluated. If it evaluates to false the right-hand side will be evaluated and
`currency` will be set to `DEFAULT_CURRENCY`.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
f4854c5 to
454b1bf
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6132 +/- ##
==========================================
+ Coverage 66.98% 67.22% +0.23%
==========================================
Files 118 118
Lines 18192 18180 -12
Branches 3079 3079
==========================================
+ Hits 12186 12221 +35
+ Misses 5347 5299 -48
- Partials 659 660 +1
🚀 New features to boost your workflow:
|
5035cb2 to
28533fb
Compare
6f36af0 to
686a897
Compare
4a7172a to
e6aa791
Compare
e6aa791 to
e30f7fb
Compare
This reverts commit e30f7fb.
ab91cf7 to
24f9389
Compare
henry-oberholtzer
left a 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.
Looks great to me - and confuse looks to be all set now too!
semohr
left a 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.
I think for the 3.13 update we don't necessarily need to wait for confuse but it might still be a good idea to update it now, if just for the improved type hints.
94e57b1 to
cbd74b3
Compare
That's exactly why I updated it for now! |
Fixes #5575
Fixes #5822
Fixes #6082
Fixes #6026
TODO
confusePython 3.13 compatibility
librosadependency from^0.10.2.post1to>=0.11where a bug withnumpytypes is fixed.audioreaddependency which now pulls instandard-aifc,standard-sunau, andaudioop-ltspackages for Python 3.13 and above.Python 3.14 compatibility
fnctl.ioctlfunction which we used to detect the terminal width. I replaced it with high-level, cross-platformshutil.get_terminal_size().librosadependencies on Python 3.14. It should work fine for people that do not useautobpm, and it may even work for those that do - if they have the right set of system dependencies available. We can revise this once we drop Python 3.9 in a couple of days.Dependency updates