Skip to content

Conversation

@lysnikolaou
Copy link
Contributor

@lysnikolaou lysnikolaou commented Jan 13, 2025

What do these changes do?

  • Change Cython dependency to 3.1.0+ for CPython 3.13 free-threading to support free-threading
  • Test the free-threaded build on CI
  • Compile Cython to C++ (instead of C) and change to a sequentially consistent atomic integer for frozen .

Are there changes in behavior for the user?

No.

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes
  • If you provide code modifications, please add yourself to CONTRIBUTORS.txt
    • The format is <Name> <Surname>.
    • Please keep the list in alphabetical order, the file is sorted by name.
  • Add a new news fragment into the CHANGES folder
    • name it <issue_id>.<type> for example (588.bugfix)
    • if you don't have an issue_id change it to the pr id after creating the pr
    • ensure type is one of the following:
      • .feature: Signifying a new feature.
      • .bugfix: Signifying a bug fix.
      • .doc: Signifying a documentation improvement.
      • .removal: Signifying a deprecation or removal of public API.
      • .misc: A ticket has been closed, but it is not of interest to users.
    • Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

lysnikolaou and others added 4 commits February 17, 2025 20:31
- This requires Cython nightly, so build configs have been changed
  to accomodate that.
- Reverted some of the changes that weren't related to free-threading
@lysnikolaou
Copy link
Contributor Author

If someone could hit Approve for the workflows to run, that'd be really helpful.

@bdraco
Copy link
Member

bdraco commented Feb 21, 2025

CI Run.

If you need me to start it, feel free to reach out on discord or matrix as I likely won't see GitHub notifications as they tend to be overwhelming

@lysnikolaou
Copy link
Contributor Author

I changed the approach here to use C++ atomics instead of critical sections. That should be faster / more scalable. If you can hit the green button again for the actions to run, that'd be great!

If you need me to start it, feel free to reach out on discord or matrix as I likely won't see GitHub notifications as they tend to be overwhelming

Not sure what Discord server you're talking about. I'd love an invitation if possible.

@bdraco
Copy link
Member

bdraco commented Mar 11, 2025

I changed the approach here to use C++ atomics instead of critical sections. That should be faster / more scalable. If you can hit the green button again for the actions to run, that'd be great!

If you need me to start it, feel free to reach out on discord or matrix as I likely won't see GitHub notifications as they tend to be overwhelming

Not sure what Discord server you're talking about. I'd love an invitation if possible.

aio-libs uses Matrix https://matrix.to/#/#aio-libs:matrix.org

However I'm usually a lot easier to find on Home Assistant's Discord: https://discord.com/invite/home-assistant

@codecov
Copy link

codecov bot commented Mar 11, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.22%. Comparing base (c28f32d) to head (e54fe07).
Report is 36 commits behind head on master.

Files with missing lines Patch % Lines
packaging/pep517_backend/_backend.py 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #618      +/-   ##
==========================================
- Coverage   96.94%   96.22%   -0.73%     
==========================================
  Files          16       14       -2     
  Lines        1278     1139     -139     
  Branches       90       90              
==========================================
- Hits         1239     1096     -143     
- Misses         14       17       +3     
- Partials       25       26       +1     
Flag Coverage Δ
CI-GHA 96.13% <83.33%> (-0.66%) ⬇️
MyPy 72.66% <83.33%> (-0.04%) ⬇️
OS-Linux 99.20% <ø> (-0.49%) ⬇️
OS-Windows 99.20% <ø> (-0.40%) ⬇️
OS-macOS 99.20% <ø> (ø)
Py-3.10.11 99.20% <ø> (-0.20%) ⬇️
Py-3.10.16 99.20% <ø> (-0.49%) ⬇️
Py-3.11.11 99.20% <ø> (-0.49%) ⬇️
Py-3.11.9 99.20% <ø> (-0.20%) ⬇️
Py-3.12.9 99.20% <ø> (-0.35%) ⬇️
Py-3.13.2 99.20% <ø> (-0.35%) ⬇️
Py-3.13.2t 99.20% <ø> (?)
Py-3.9.13 99.20% <ø> (-0.20%) ⬇️
Py-3.9.21 99.20% <ø> (-0.49%) ⬇️
Py-pypy7.3.16 98.41% <ø> (ø)
Py-pypy7.3.19 98.41% <ø> (ø)
VM-macos-latest 99.20% <ø> (ø)
VM-ubuntu-latest 99.20% <ø> (-0.49%) ⬇️
VM-windows-latest 99.20% <ø> (-0.40%) ⬇️
pytest 99.20% <ø> (-0.35%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lysnikolaou
Copy link
Contributor Author

This is ready for another round of reviews, whenever anyone finds some spare cycles.

Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. PR description needs to be updated to reflect the changes.

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one thing that bothers me.

lysnikolaou and others added 2 commits March 28, 2025 16:03
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lysnikolaou

@bdraco bdraco merged commit f545c23 into aio-libs:master Mar 28, 2025
45 of 48 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants