Skip to content

don't use cythonize in setup.py #379

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

Open
jswhit wants to merge 6 commits into
base: master
Choose a base branch
from
Open

don't use cythonize in setup.py #379

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9710ef9
don't use cythonize in setup.py
Oct 14, 2025
ffa6f29
update
Oct 14, 2025
6bdd0ff
update
Oct 14, 2025
45cfe01
add setuptools[scm] requirement
Oct 14, 2025
1214424
remove len(sys.argv)
Oct 15, 2025
14f4e48
update
Oct 15, 2025
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
4 changes: 2 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@ readme = "README.md"
authors = [
{ name="Jeff Whitaker", email="[email protected]"}
]
license = "MIT"
license-files = ["LICENSE"]
license = {text = "MIT"}
classifiers = [
'Development Status :: 5 - Production/Stable',
'Operating System :: MacOS :: MacOS X',
Expand Down Expand Up @@ -41,6 +40,7 @@ dev = [
[build-system]
requires = [
"setuptools>=77.0.1",
"setuptools_scm[toml]>=3.4",
"cython>=0.29.20",
"numpy>=2.0.0,<3",
]
Expand Down
26 changes: 13 additions & 13 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import sys
import numpy

from Cython.Build import cythonize
from setuptools import Command, Extension, setup


Expand All @@ -28,6 +27,7 @@
NAME = 'cftime'
CFTIME_DIR = os.path.join(SRCDIR, NAME)
CYTHON_FNAME = os.path.join(CFTIME_DIR, '_{}.pyx'.format(NAME))
CYTHON_CNAME = os.path.join(CFTIME_DIR, '_{}.c'.format(NAME))


class CleanCython(Command):
Expand All @@ -53,8 +53,7 @@ def run(self):
print('clean: skipping file {!r}'.format(artifact))


if ((FLAG_COVERAGE in sys.argv or os.environ.get('CYTHON_COVERAGE', None))
and cythonize):
if FLAG_COVERAGE in sys.argv or os.environ.get('CYTHON_COVERAGE', None):
COMPILER_DIRECTIVES = {
**COMPILER_DIRECTIVES, **COVERAGE_COMPILER_DIRECTIVES
}
Expand All @@ -68,16 +67,17 @@ def run(self):
if any([arg in CMDS_NOCYTHONIZE for arg in sys.argv]):
ext_modules = []
else:
extension = Extension('{}._{}'.format(NAME, NAME),
sources=[os.path.relpath(CYTHON_FNAME, BASEDIR)],
define_macros=DEFINE_MACROS,
include_dirs=[numpy.get_include(),])

ext_modules = cythonize(
extension,
compiler_directives=COMPILER_DIRECTIVES,
language_level=3,
)
ext_modules = [Extension('{}._{}'.format(NAME, NAME),
sources=[os.path.relpath(CYTHON_FNAME, BASEDIR)],
define_macros=DEFINE_MACROS,
include_dirs=[numpy.get_include(),])]
for e in ext_modules:
e.cython_directives = {'language_level': "3"}
e.compiler_directives = COMPILER_DIRECTIVES
# remove _cftime.c file if it exists, so cython will recompile _cftime.pyx.
if len(sys.argv) >= 2:
Copy link
Contributor

@neutrinoceros neutrinoceros Oct 15, 2025

Choose a reason for hiding this comment

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

not sure what the intention for this check is. Seems to me there are many ways to end up executing this code with completely different commands so I'm a bit unconfortable with it. Can you explain ?

Copy link
Collaborator Author

@jswhit jswhit Oct 15, 2025

Choose a reason for hiding this comment

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

the idea is that anytime you run setup.py, you will want to recompile _cftime.pyx. If there is a left-over _cftime.c file, then cython won't run, so this code removes it if it exists.

Copy link
Collaborator Author

@jswhit jswhit Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

From the setuptools docs:

"When your Cython extension modules are declared using the setuptools.Extension class, setuptools will detect at build time whether Cython is installed or not.

If Cython is present, then setuptools will use it to build the .pyx files. Otherwise, setuptools will try to find and compile the equivalent .c files (instead of .pyx). These files can be generated using the cython command line tool."

so you don't really need to use cythonize explicitly.

Copy link
Contributor

@neutrinoceros neutrinoceros Oct 15, 2025

Choose a reason for hiding this comment

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

the idea is that anytime you run setup.py, you will want to recompile _cftime.pyx

but why would this be captured specifically by the number of CLI arguments ? This is what I'm having a hard time wrapping my head around.

Copy link
Collaborator Author

@jswhit jswhit Oct 15, 2025

Choose a reason for hiding this comment

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

I copied this from netcdf4-python - I think it can be removed.

if os.path.exists(CYTHON_CNAME):
os.remove(CYTHON_CNAME)

setup(
cmdclass={'clean_cython': CleanCython},
Expand Down
Loading