Skip to content
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

add PyStringMethods::encode_utf8 #3801

Merged
merged 1 commit into from
Feb 10, 2024
Merged

Conversation

davidhewitt
Copy link
Member

This PR adds an encode_utf8() method to explicitly convert Bound<PyString> to Bound<PyBytes>, equivalent to the Python s.encode('utf-8').

This turns out to be useful in #3708 and I felt like it made sense for this to be a public API.

@davidhewitt
Copy link
Member Author

davidhewitt commented Feb 5, 2024

I chose .encode_utf8() because I think in principle we could have .encode("some_encoding"), but I would prefer to punt on that more complete option as a future addition.

Copy link

codspeed-hq bot commented Feb 5, 2024

CodSpeed Performance Report

Merging #3801 will improve performances by 20.41%

Comparing davidhewitt:encode-utf8 (662eecf) with main (ecb4ecb)

Summary

⚡ 3 improvements
✅ 76 untouched benchmarks

Benchmarks breakdown

Benchmark main davidhewitt:encode-utf8 Change
mapping_from_dict 327.8 ns 272.2 ns +20.41%
sequence_from_list 355.6 ns 300 ns +18.52%
not_a_list_via_downcast 242.8 ns 215 ns +12.92%

Copy link
Contributor

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

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

This looks good to me. I agree with you that encode is better used for a more generic encoding method. Also encode_utf8 makes it very clear which encoding is used.

@davidhewitt davidhewitt added this pull request to the merge queue Feb 10, 2024
Merged via the queue into PyO3:main with commit fa53d81 Feb 10, 2024
36 checks passed
@davidhewitt davidhewitt deleted the encode-utf8 branch February 10, 2024 14:55
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.

2 participants