Skip to content

Parallel Cythonization for spawned Processes #30862

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.

Sign up for GitHub

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 5 commits into from
Jan 24, 2020
Merged

Parallel Cythonization for spawned Processes #30862

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
292f941
Parallel compilation for windows
WillAyd Jan 9, 2020
2ca1b03
removed unneeded check
WillAyd Jan 9, 2020
82336eb
Merge remote-tracking branch 'upstream/master' into windows-parallel-…
WillAyd Jan 20, 2020
1eeb889
Merge remote-tracking branch 'upstream/master' into windows-parallel-…
WillAyd Jan 20, 2020
64db610
setup_package func
WillAyd Jan 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 36 additions & 36 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import argparse
from distutils.sysconfig import get_config_vars
from distutils.version import LooseVersion
import multiprocessing
import os
from os.path import join as pjoin
import platform
Expand Down Expand Up @@ -531,11 +532,6 @@ def maybe_cythonize(extensions, *args, **kwargs):
elif parsed.j:
nthreads = parsed.j

# GH#30356 Cythonize doesn't support parallel on Windows
if is_platform_windows() and nthreads > 0:
print("Parallel build for cythonize ignored on Windows")
nthreads = 0

kwargs["nthreads"] = nthreads
build_ext.render_templates(_pxifiles)
return cythonize(extensions, *args, **kwargs)
Expand Down Expand Up @@ -747,34 +743,38 @@ def srcpath(name=None, suffix=".pyx", subdir="src"):
# The build cache system does string matching below this point.
# if you change something, be careful.

setup(
name=DISTNAME,
maintainer=AUTHOR,
version=versioneer.get_version(),
packages=find_packages(include=["pandas", "pandas.*"]),
package_data={"": ["templates/*", "_libs/*.dll"]},
ext_modules=maybe_cythonize(extensions, compiler_directives=directives),
maintainer_email=EMAIL,
description=DESCRIPTION,
license=LICENSE,
cmdclass=cmdclass,
url=URL,
download_url=DOWNLOAD_URL,
project_urls=PROJECT_URLS,
long_description=LONG_DESCRIPTION,
classifiers=CLASSIFIERS,
platforms="any",
python_requires=">=3.6.1",
extras_require={
"test": [
# sync with setup.cfg minversion & install.rst
"pytest>=4.0.2",
"pytest-xdist",
"hypothesis>=3.58",
]
},
entry_points={
"pandas_plotting_backends": ["matplotlib = pandas:plotting._matplotlib"]
},
**setuptools_kwargs,
)
if __name__ == "__main__":
# Freeze to support parallel compilation when using spawn instead of fork
multiprocessing.freeze_support()
Copy link
Contributor

Choose a reason for hiding this comment

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

why does this need to be here, why wouldn't you just put this at the top;

the checking if __main__ seems very odd (in setup.py), I have never seen this, can you point to an authoritative reference for this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just for some other examples - numpy does something like this:

https://github.com/numpy/numpy/blob/a16dfb5b397718c5caf32c63f024d1ce3518c5a1/setup.py#L484

likewise with scikit-learn:

https://github.com/scikit-learn/scikit-learn/blob/00fe3d6944f91d52b24d0f59cc9a4dd83be99bcf/setup.py#L302

I can move to the top but I think would be weird to include everything in that; especially with everything that is currently global

Copy link
Contributor

Choose a reason for hiding this comment

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

gotcha, can you change to the setup_package pattern

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good - just did in latest. We probably could/should make it so that there aren't any global variables in the setup file, but it would be a rather large undertaking so kept most globals where they are for now


setup(
name=DISTNAME,
maintainer=AUTHOR,
version=versioneer.get_version(),
packages=find_packages(include=["pandas", "pandas.*"]),
package_data={"": ["templates/*", "_libs/*.dll"]},
ext_modules=maybe_cythonize(extensions, compiler_directives=directives),
maintainer_email=EMAIL,
description=DESCRIPTION,
license=LICENSE,
cmdclass=cmdclass,
url=URL,
download_url=DOWNLOAD_URL,
project_urls=PROJECT_URLS,
long_description=LONG_DESCRIPTION,
classifiers=CLASSIFIERS,
platforms="any",
python_requires=">=3.6.1",
extras_require={
"test": [
# sync with setup.cfg minversion & install.rst
"pytest>=4.0.2",
"pytest-xdist",
"hypothesis>=3.58",
]
},
entry_points={
"pandas_plotting_backends": ["matplotlib = pandas:plotting._matplotlib"]
},
**setuptools_kwargs,
)