Skip to content
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

Jump to bottom

pep518: purge legacy numpy workaround in setup.py #361

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

pep518: purge legacy numpy workaround in setup.py #361

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 16 additions & 29 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
import sys
import warnings
from distutils.sysconfig import get_config_var
from os import environ
from pathlib import Path
from shutil import copy

# safe to import numpy here thanks to pep518
import numpy as np
from setuptools import Command, Extension, setup

# Default to using cython, but use the .c files if it doesn't exist
Copy link
Member

Choose a reason for hiding this comment

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

Why would we want to have a build-time dependency on numpy, but an optional build-time dependency on Cython. Surely we can now move to a world where we unconditionally depend on Cython for building from source (and sdist)?

try:
from Cython.Build import cythonize
except ImportError:
cythonize = False
wmsg = "Cython unavailable, unable to cythonize cf-units extensions!"
warnings.warn(wmsg)
cythonize = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
cythonize = None
cythonize = False

Previous was clearer in IMO


COMPILER_DIRECTIVES = {}
DEFINE_MACROS = None
Expand Down Expand Up @@ -46,6 +51,7 @@ def get_include_dirs():
include_dir = get_config_var("INCLUDEDIR")
if include_dir is not None:
include_dirs.append(include_dir)
include_dirs.append(np.get_include())
return include_dirs


Expand Down Expand Up @@ -97,28 +103,11 @@ def get_package_data():
return package_data


def numpy_build_ext(pars):
from setuptools.command.build_ext import build_ext as _build_ext

class build_ext(_build_ext):
def finalize_options(self):
# See https://github.com/SciTools/cf-units/issues/151
def _set_builtin(name, value):
if isinstance(__builtins__, dict):
__builtins__[name] = value
else:
setattr(__builtins__, name, value)

_build_ext.finalize_options(self)
_set_builtin("__NUMPY_SETUP__", False)
import numpy

self.include_dirs.append(numpy.get_include())

return build_ext(pars)

cython_coverage_enabled = (
environ.get("CYTHON_COVERAGE", None) or FLAG_COVERAGE in sys.argv
)

if FLAG_COVERAGE in sys.argv or environ.get("CYTHON_COVERAGE", None):
if cythonize and cython_coverage_enabled:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why has this changed from or to and?

COMPILER_DIRECTIVES = {"linetrace": True}
DEFINE_MACROS = [("CYTHON_TRACE", "1"), ("CYTHON_TRACE_NOGIL", "1")]
if FLAG_COVERAGE in sys.argv:
Expand All @@ -144,12 +133,10 @@ def _set_builtin(name, value):
udunits_ext, compiler_directives=COMPILER_DIRECTIVES, language_level=2
)

cmdclass = {"clean_cython": CleanCython, "build_ext": numpy_build_ext}

kwargs = dict(
cmdclass=cmdclass,
ext_modules=[udunits_ext],
package_data=get_package_data(),
)
kwargs = {
"cmdclass": {"clean_cython": CleanCython},
"ext_modules": [udunits_ext],
"package_data": get_package_data(),
}

setup(**kwargs)
Loading