Skip to content

Use CPython stable APIs for implementing tuples. #1129

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 1 commit into from
Aug 31, 2020

Conversation

alex
Copy link
Contributor

@alex alex commented Aug 31, 2020

Refs #1125

Reduces the number of compilation failures with --features abi3 from 56 to 53.

I chose to implement this by simple replacing all functions with their stable equivilant. This potentially regresses performance somewhat. If it's preferable, I can switch which call is used based on the #[cfg] at the price of some verbosity.

@kngwyu
Copy link
Member

kngwyu commented Aug 31, 2020

How about making a single specific branch for this instead of merging series of PRs to the master?
If you are OK, I will create a separate branch (e.g., pyo3/abi3) and you send PRs to that branch.

@alex
Copy link
Contributor Author

alex commented Aug 31, 2020

I personally find it easier to work in small PRs that can each be reviewed by themselves. This ensures that I'm constantly working off the latest version and didn't break anything.

The next PR is probably going to be type definition support, which will be the biggest part of this anyways :-)

@alex
Copy link
Contributor Author

alex commented Aug 31, 2020

Coverage change is because as_slice() lost coverage since it's no longer used in iter(). Let me write a test for it.

@kngwyu
Copy link
Member

kngwyu commented Aug 31, 2020

The next PR is probably going to be type definition support, which will be the biggest part of this anyways :-)

That patch would require careful design decisions and it's easier to work for me if we have an upstream branch for it.

@alex
Copy link
Contributor Author

alex commented Aug 31, 2020

Yes, I agree that PR will be very subtle and require careful thought! Is it ok to review/merge this tuple PR as is, and then we can do a branch for the type definition work?

Copy link
Member

@kngwyu kngwyu left a comment

Choose a reason for hiding this comment

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

Thanks, mostly LGTM.

@kngwyu
Copy link
Member

kngwyu commented Aug 31, 2020

Is it ok to review/merge this tuple PR as is, and then we can do a branch for the type definition work?

Of course. I'll create pyo3/abi3 branch and please open a PR to that branch first. Then let's work on the branch collaboratory.

Refs PyO3#1125

Reduces the number of compilation failures with `--features abi3` from 56 to 53.
@alex
Copy link
Contributor Author

alex commented Aug 31, 2020

Ok, should be good now!

@kngwyu kngwyu merged commit 4a05f27 into PyO3:master Aug 31, 2020
@kngwyu
Copy link
Member

kngwyu commented Aug 31, 2020

Now, https://github.com/PyO3/pyo3/tree/abi3 is available.

@alex alex deleted the limit-tuples branch August 31, 2020 13:17
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