Skip to content

Replace internal API _PyBytes_Join, removed in Python 3.13 #31

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

Merged
merged 2 commits into from
Jan 23, 2024

Conversation

musicinmybrain
Copy link
Contributor

@musicinmybrain musicinmybrain commented Jan 23, 2024

Checklist

  • Pull request details were added to CHANGELOG.rst
  • Documentation was updated (if needed)

I know that the regex package wrote a somewhat nontrivial replacement routine in mrabarnett/mrab-regex@bc73ebb, but this call happens only once at the end of GzipReader_readall, so I expect any added overhead should be negligible. Similarly, we could have used something like PyObject_CallMethodObjArgs, but that would have required us to build a PyObject containing the method name. In a quick test, I didn’t observe any change in the overall time required to run the test suite.

Fixes #30.

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (dc647e2) 94.43% compared to head (1c63077) 94.43%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop      #31   +/-   ##
========================================
  Coverage    94.43%   94.43%           
========================================
  Files            3        3           
  Lines          449      449           
  Branches        81       81           
========================================
  Hits           424      424           
  Misses          15       15           
  Partials        10       10           

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

@rhpvorderman
Copy link
Contributor

Thanks for submitting the patch! This looks like the most straightforward way to fix this. I will merge once the tests pass.

@rhpvorderman rhpvorderman merged commit 37011be into pycompression:develop Jan 23, 2024
@rhpvorderman
Copy link
Contributor

I did a small benchmark run and I see no big differences indeed. Thanks for the patch again!

@musicinmybrain
Copy link
Contributor Author

I did a small benchmark run and I see no big differences indeed. Thanks for the patch again!

You’re welcome! Glad it works for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Internal API _PyBytes_Join() is removed in Python 3.13
2 participants