-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add step to Cythonize parts of Distributed #33
Conversation
echo "Cythonize Distributed" | ||
pushd "${CONDA_PREFIX}/lib/python3.8/site-packages/" | ||
cythonize -i \ | ||
"distributed/protocol/serialize.py" |
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.
This already has some bits for Cython (though not everywhere). So went ahead and Cythonized it. Though if it causes issues for some reason, we can skip this as it is less critical atm.
pushd "${CONDA_PREFIX}/lib/python3.8/site-packages/" | ||
cythonize -i \ | ||
"distributed/protocol/serialize.py" | ||
"distributed/scheduler.py" |
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.
We haven't made any changes here yet, but it seems like a good idea to queue this up so we are ready once we do.
Also please let me know if we want a cleanup step at the end. That should be easy to add. |
Seems to be working:
|
Thanks @jakirkham ! Merging in |
Thanks Ben! 😄 This part seems interesting. What are the permissions on that file?
Edit: NVM see the issue. Explained below ( #33 (review) ) with a fix in PR ( #34 ). |
cythonize -i \ | ||
"distributed/protocol/serialize.py" | ||
"distributed/scheduler.py" |
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.
Oops forgot a \
. Fixing with PR ( #34 ).
Adds a step to the nightly benchmarks to Cythonize some modules in Distributed in-place. We go ahead and do this in the environment's
site-packages
as this environment exists for this benchmark work. So altering it this way shouldn't present an issue.