Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PySequence: use usize everywhere, fix in-place methods #1803

Merged
merged 5 commits into from
Aug 24, 2021

Conversation

birkenfeld
Copy link
Member

No description provided.

@davidhewitt
Copy link
Member

(I guess this needs to wait for #1802)

birkenfeld added a commit that referenced this pull request Aug 18, 2021
@birkenfeld birkenfeld changed the title PySequence: use usize everywhere PySequence: use usize everywhere, fix in-place methods Aug 18, 2021
@davidhewitt
Copy link
Member

👍 this is looking good!

It looks like there's a few PySequence apis which are lacking tests; would you be willing to add them as part of this PR?

a) not leak a reference
b) correctly return the result, since for immutable types `self` is not actually mutated in place
@birkenfeld
Copy link
Member Author

Sure.

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.

Looks good to me, thanks for the new tests!

I guess still to be added for #1667 is to add the impl std::ops::Index implementations, and maybe also an entry in the migration guide.

@davidhewitt davidhewitt merged commit cfd6a5b into main Aug 24, 2021
@birkenfeld birkenfeld deleted the pysequence_usize branch August 24, 2021 06:47
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants