Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Complete removal of PyCXX#3723
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
jenshnielsen commentedOct 26, 2014
It looks good to me. I did a quick test locally and everything seems to work fine. 👍 on merging this from me. |
MNT : Complete removal of PyCXX
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think you need toPy_DECREF(segs) here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, well spotted.
mdboom commentedOct 27, 2014
Thanks for doing this! I appreciate how much work this was. Sorry about commenting after this was merged, but it looks like this was up for less than 24 hours! |
tacaswell commentedOct 27, 2014
Sorry, I applied the logic of the other pycxx branch and merged it on trusting the test suite. |
mdboom commentedOct 27, 2014
@tacaswell: No problem. It's more important that this works than anything else, so merging early is the best for that. |
ianthomas23 commentedOct 27, 2014
@tacaswell: What should I do now that this PR has been merged? Should I just write a new PR for the changes and refer back to this one? |
tacaswell commentedOct 27, 2014
Yes, that is probably the best way On Mon, Oct 27, 2014 at 3:06 PM, Ian Thomasnotifications@github.com
Thomas Caswell |
Final decxx corrections to PR#3723
This PR completes the removal of PyCXX from matplotlib following on from PR#3646.
There are 2 commits. The first converts the _tri module, the last module to use PyCXX, to use the Python/C API instead. I've followed the lead of#3646 so that we have some code consistency in terms of separate wrapper files, code layout, use of the same exception-catching macros, etc.
The second commit removes the PyCXX source code, the relevant sections of the setup scripts, and a few other places where it was mentioned.
It is always nice to write a PR that is such a net reduction in code!