-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
Pure-Python approach to closure cell rewriting #522
Conversation
…only defined for their side effects on closure cells
Interesting. I originally wrote the ctypes approach, but I used ctypes because I was not aware of a different way of doing it. If the end result is the same and we stop using ctypes, I think that'd be a win. |
One-month checkin on this? The end result is the same as far as I can tell. |
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.
looks great
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.
Thanks I've procrastinated on it because I wasn't comfortable to review this amazing hack myself. I've just hijacked @markshannon at the core sprints to have a look.
Thank you very much, let's hope this is the last we hear about cells. ;)
Looks like this is breaking on 3.8 😞:
|
Instead of using ctypes, make a function that sets a cellvar and then fiddle with the code object to make it set a freevar instead. PyPy still uses
__setstate__
because it is easy and fast. I believe this is similar to the approach used by thecloudpickle
package, but I didn't directly reference their code when writing this.The performance of the new approach is comparable to the old one; timeit on py2.7 gives 645ns for the new approach (down from 691ns for the old), and on py3.6 gives 822ns for the new approach (up from 659ns for the old). And dropping the use of ctypes seems like a win.
Pull Request Check List
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!
If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.
.pyi
). [N/A]tests/typing_example.py
. [N/A]docs/api.rst
by hand. [N/A]@attr.s()
have to be added by hand too. [N/A]versionadded
,versionchanged
, ordeprecated
directives. [N/A].rst
files is written using semantic newlines.changelog.d
.If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!