Skip to content

add Bound constructors for PyByteArray and PyMemoryView#3786

Merged
davidhewitt merged 2 commits intoPyO3:mainfrom
Icxolu:bytearray
Jan 31, 2024
Merged

add Bound constructors for PyByteArray and PyMemoryView#3786
davidhewitt merged 2 commits intoPyO3:mainfrom
Icxolu:bytearray

Conversation

@Icxolu
Copy link
Member

@Icxolu Icxolu commented Jan 30, 2024

Part of #3684

The adds the Bound constructors for PyByteArray and PyMemoryView.

Since the original from constructor used as_ptr(), which returns a borrowed ptr, I used a borrowed &Bound for the new one. Please correct me if I'm wrong here.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 30, 2024

CodSpeed Performance Report

Merging #3786 will improve performances by 12.92%

Comparing Icxolu:bytearray (b14dbcf) with main (6040d93)

Summary

⚡ 1 improvements
✅ 78 untouched benchmarks

Benchmarks breakdown

Benchmark main Icxolu:bytearray Change
not_a_list_via_downcast 242.8 ns 215 ns +12.92%

@davidhewitt davidhewitt added the CI-skip-changelog Skip checking changelog entry label Jan 30, 2024
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Brilliant, thank you yet again! Good pick to choose these ones; I'd totally forgotten they needed constructors added still.

Since the original from constructor used as_ptr(), which returns a borrowed ptr, I used a borrowed &Bound for the new one. Please correct me if I'm wrong here.

&Bound is exactly right based on what we've been doing elsewhere 👍

Just one tiny detail which is a suggestion to improve the surrounding code a tiny bit while we're there...

Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Perfect, thanks again!

@davidhewitt davidhewitt added this pull request to the merge queue Jan 31, 2024
@davidhewitt
Copy link
Member

Btw, if you are interested in some reviewing as well as coding, I've started keeping a list on #3684, some help getting those reviewed and merged would go a long way towards getting this new API shipped!

Merged via the queue into PyO3:main with commit aa1a986 Jan 31, 2024
@Icxolu
Copy link
Member Author

Icxolu commented Jan 31, 2024

Btw, if you are interested in some reviewing as well as coding, I've started keeping a list on #3684, some help getting those reviewed and merged would go a long way towards getting this new API shipped!

I'll have a look and leave some comments.

@Icxolu Icxolu deleted the bytearray branch January 31, 2024 17:25
@Icxolu Icxolu mentioned this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants