From e23ec4ba36e3f705e1ebf34337f530209652a9d0 Mon Sep 17 00:00:00 2001 From: Joseph Hunkeler Date: Wed, 6 Jan 2021 16:35:44 -0500 Subject: [PATCH 1/3] Fix build system Use static docstring Update pyproject.toml Update MANIFEST.in Add headers to manifest Remove unused docstring generator Force --no-deps builds to work Remove C source .gitignore files Co-authored-by: P. L. Lim <2090236+pllim@users.noreply.github.com> --- .github/workflows/codeql-analysis.yml | 2 +- MANIFEST.in | 2 +- pyproject.toml | 4 +- setup.cfg | 8 -- setup.py | 31 +++++-- synphot/docstrings.py | 39 --------- synphot/include/.gitignore | 1 - synphot/include/synphot_utils.h | 38 +++++++++ synphot/setup_package.py | 117 -------------------------- synphot/src/.gitignore | 5 -- synphot/src/synphot_utils.c | 3 +- tox.ini | 4 - 12 files changed, 65 insertions(+), 189 deletions(-) delete mode 100644 synphot/docstrings.py delete mode 100644 synphot/include/.gitignore create mode 100644 synphot/include/synphot_utils.h delete mode 100644 synphot/setup_package.py delete mode 100644 synphot/src/.gitignore diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 9803674b..b8991589 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -94,7 +94,7 @@ jobs: if: matrix.language == 'cpp' run: | pip install -U pip setuptools_scm wheel - pip install extension-helpers numpy astropy + pip install numpy astropy python setup.py build_ext --inplace - name: Perform CodeQL Analysis diff --git a/MANIFEST.in b/MANIFEST.in index 8881739f..922d52b6 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -5,7 +5,7 @@ include CITATION.md include setup.cfg include pyproject.toml -include synphot/include/.gitignore +recursive-include synphot/include *.h recursive-include synphot *.c recursive-include docs * diff --git a/pyproject.toml b/pyproject.toml index 70365621..55aee82f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,6 +2,6 @@ requires = ["setuptools>=30.3.0", "setuptools_scm", "wheel", - "oldest-supported-numpy", - "extension-helpers"] + "oldest-supported-numpy" + ] build-backend = "setuptools.build_meta" diff --git a/setup.cfg b/setup.cfg index 485e26e4..fa20999a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -59,14 +59,6 @@ docs = [options.package_data] synphot.tests = data/* -[extension=synphot.synphot_utils] -optional = True -fail_message = - ********************************************************* - Build failed, trying without C extension. - This removes no functionality but may affect performance. - ********************************************************* - [flake8] # Ignoring these for now: # I100: import statements are in the wrong order diff --git a/setup.py b/setup.py index 2e0f245a..7f9bb871 100755 --- a/setup.py +++ b/setup.py @@ -1,18 +1,31 @@ #!/usr/bin/env python # Licensed under a 3-clause BSD style license - see LICENSE.rst +import os import sys -from setuptools import setup +from setuptools import setup, Extension -try: - from extension_helpers import get_extensions -except ModuleNotFoundError: - print(""" -extension-helpers is not detected, please install with: +LOCALROOT = 'synphot' + + +def get_extensions(): + from collections import defaultdict + try: + import numpy + except ImportError as e: + print(e, file=sys.stdout) + return [] + + cfg = defaultdict(list) + cfg['include_dirs'].extend([ + numpy.get_include(), + os.path.join(LOCALROOT, "include")]) + cfg['sources'] = [ + os.path.join(LOCALROOT, 'src', 'synphot_utils.c')] + cfg = dict((str(key), val) for key, val in cfg.items()) + + return [Extension('synphot.synphot_utils', **cfg)] - pip install -e . -""") - sys.exit(1) TEST_HELP = """ Note: running tests is no longer done using 'python setup.py test'. Instead diff --git a/synphot/docstrings.py b/synphot/docstrings.py deleted file mode 100644 index a8ef708b..00000000 --- a/synphot/docstrings.py +++ /dev/null @@ -1,39 +0,0 @@ -# Licensed under a 3-clause BSD style license - see LICENSE.rst -"""It gets to be really tedious to type long docstrings in ANSI C -syntax (since multi-line string literals are not valid). -Therefore, the docstrings are written here, which are then -converted by setup.py into include/docstrings.h, which is -included by src/synphot_utils.c. - -""" - - -calcbinflux = """ -calcbinflux(len_binwave, i_beg, i_end, avflux, deltaw) - -Sum over each bin. - -Parameters ----------- -len_binwave : int - Number of wavelength bin centers. - -i_beg, i_end : array-like - Locations of bin edges in ``deltaw``. - -avflux : array-like - Average flux associated with ``deltaw``. - -deltaw : array-like - Delta of merge wavelengths (native + centers + edges). - Values are in ascending order. - -Returns -------- -binflux : array-like - Integrated flux associated with given bins in ascending order. - -intwave : array-like - Integrated delta wavelength associated with ``binflux``. - -""" diff --git a/synphot/include/.gitignore b/synphot/include/.gitignore deleted file mode 100644 index af44f9fb..00000000 --- a/synphot/include/.gitignore +++ /dev/null @@ -1 +0,0 @@ -docstrings.h diff --git a/synphot/include/synphot_utils.h b/synphot/include/synphot_utils.h new file mode 100644 index 00000000..ac1b4fd9 --- /dev/null +++ b/synphot/include/synphot_utils.h @@ -0,0 +1,38 @@ +#ifndef SYNPHOT_UTILS__H +#define SYHPHOT_UTILS__H + +#define doc_calcbinflux \ +"\ +\n\ +\n\ +\n\ +calcbinflux(len_binwave, i_beg, i_end, avflux, deltaw)\n\ +\n\ +Sum over each bin.\n\ +\n\ +Parameters\n\ +----------\n\ +len_binwave : int\n\ + Number of wavelength bin centers.\n\ +\n\ +i_beg, i_end : array-like\n\ + Locations of bin edges in ``deltaw``.\n\ +\n\ +avflux : array-like\n\ + Average flux associated with ``deltaw``.\n\ +\n\ +deltaw : array-like\n\ + Delta of merge wavelengths (native + centers + edges).\n\ + Values are in ascending order.\n\ +\n\ +Returns\n\ +-------\n\ +binflux : array-like\n\ + Integrated flux associated with given bins in ascending order.\n\ +\n\ +intwave : array-like\n\ + Integrated delta wavelength associated with ``binflux``.\n\ +\n\ +\0" + +#endif diff --git a/synphot/setup_package.py b/synphot/setup_package.py deleted file mode 100644 index 9232b047..00000000 --- a/synphot/setup_package.py +++ /dev/null @@ -1,117 +0,0 @@ -# Licensed under a 3-clause BSD style license - see LICENSE.rst - -# STDLIB -import io -import os -from setuptools import Extension - -# ASTROPY -from extension_helpers import import_file, write_if_different - -LOCALROOT = os.path.relpath(os.path.dirname(__file__)) - - -def string_escape(s): - s = s.decode('ascii').encode('ascii', 'backslashreplace') - s = s.replace(b'\n', b'\\n') - s = s.replace(b'\0', b'\\0') - return s.decode('ascii') - - -def generate_c_docstrings(): - docstrings = import_file(os.path.join(LOCALROOT, 'docstrings.py')) - docstrings = docstrings.__dict__ - keys = [ - key for key, val in docstrings.items() - if not key.startswith('__') and isinstance(val, str)] - keys.sort() - docs = {} - for key in keys: - docs[key] = docstrings[key].encode('utf8').lstrip() + b'\0' - - h_file = io.StringIO() - h_file.write("""/* -DO NOT EDIT! - -This file is autogenerated by synphot/setup_package.py. To edit -its contents, edit synphot/docstrings.py -*/ - -#ifndef __DOCSTRINGS_H__ -#define __DOCSTRINGS_H__ - -#if defined(_MSC_VER) -void fill_docstrings(void); -#endif - -""") - for key in keys: - val = docs[key] - h_file.write('extern char doc_{0}[{1}];\n'.format(key, len(val))) - h_file.write("\n#endif\n\n") - - write_if_different( - os.path.join(LOCALROOT, 'include', 'docstrings.h'), - h_file.getvalue().encode('utf-8')) - - c_file = io.StringIO() - c_file.write("""/* -DO NOT EDIT! - -This file is autogenerated by synphot/setup_package.py. To edit -its contents, edit synphot/docstrings.py - -The weirdness here with strncpy is because some C compilers, notably -MSVC, do not support string literals greater than 256 characters. -*/ - -#include -#include "docstrings.h" - -#if defined(_MSC_VER) -""") - for key in keys: - val = docs[key] - c_file.write('char doc_{0}[{1}];\n'.format(key, len(val))) - - c_file.write("\nvoid fill_docstrings(void)\n{\n") - for key in keys: - val = docs[key] - # For portability across various compilers, we need to fill the - # docstrings in 256-character chunks - for i in range(0, len(val), 256): - chunk = string_escape(val[i:i + 256]).replace('"', '\\"') - c_file.write(' strncpy(doc_{0} + {1}, "{2}", {3});\n'.format( - key, i, chunk, min(len(val) - i, 256))) - c_file.write("\n") - c_file.write("\n}\n\n") - - c_file.write("#else /* UNIX */\n") - - for key in keys: - val = docs[key] - c_file.write('char doc_{0}[{1}] = "{2}";\n\n'.format( - key, len(val), string_escape(val).replace('"', '\\"'))) - - c_file.write("#endif\n") - - write_if_different( - os.path.join(LOCALROOT, 'src', 'docstrings.c'), - c_file.getvalue().encode('utf-8')) - - -def get_extensions(): - from collections import defaultdict - import numpy - generate_c_docstrings() - - cfg = defaultdict(list) - cfg['include_dirs'].extend([ - numpy.get_include(), - os.path.join(LOCALROOT, "include")]) - cfg['sources'] = [ - os.path.join(LOCALROOT, 'src', 'synphot_utils.c'), - os.path.join(LOCALROOT, 'src', 'docstrings.c')] - cfg = dict((str(key), val) for key, val in cfg.items()) - - return [Extension('synphot.synphot_utils', **cfg)] diff --git a/synphot/src/.gitignore b/synphot/src/.gitignore deleted file mode 100644 index b9383aba..00000000 --- a/synphot/src/.gitignore +++ /dev/null @@ -1,5 +0,0 @@ -# Don't ignore *.c files in this directory tree. We don't have any -# Cython files here. - -!*.c -docstrings.c diff --git a/synphot/src/synphot_utils.c b/synphot/src/synphot_utils.c index 7e0b18cc..9bcc0f88 100644 --- a/synphot/src/synphot_utils.c +++ b/synphot/src/synphot_utils.c @@ -4,8 +4,7 @@ #include "Python.h" #include - -#include "docstrings.h" +#include "synphot_utils.h" static PyObject * py_calcbinflux(PyObject *self, PyObject *args) { diff --git a/tox.ini b/tox.ini index 0b68d4a1..fff90cbf 100644 --- a/tox.ini +++ b/tox.ini @@ -12,7 +12,6 @@ isolated_build = true # Pass through the following environemnt variables which are needed for the CI passenv = HOME WINDIR CC CI -# FIXME: Cannot do this, need to force C-ext to build below. # Run the tests in a temporary directory to make sure that we don't import # package from the source tree #changedir = .tmp/{envname} @@ -53,10 +52,7 @@ extras = alldeps: all commands = - # FIXME: Not sure why need this for tox to see the C-ext - python -m pip install extension-helpers python setup.py build_ext --inplace - # End of FIXME pip freeze !cov: pytest --pyargs synphot {toxinidir}/docs {posargs} cov: pytest --pyargs synphot {toxinidir}/docs --cov synphot --cov-config={toxinidir}/setup.cfg {posargs} From 848760aed6dbb42e967194a4bccf347af6c3d814 Mon Sep 17 00:00:00 2001 From: Pey Lian Lim <2090236+pllim@users.noreply.github.com> Date: Mon, 11 Jan 2021 16:05:28 -0500 Subject: [PATCH 2/3] Undo catching numpy import error in setup --- setup.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/setup.py b/setup.py index 7f9bb871..0465311e 100755 --- a/setup.py +++ b/setup.py @@ -10,11 +10,7 @@ def get_extensions(): from collections import defaultdict - try: - import numpy - except ImportError as e: - print(e, file=sys.stdout) - return [] + import numpy cfg = defaultdict(list) cfg['include_dirs'].extend([ From 6228183eb4d9e8d1a4866244107f24bf6b949945 Mon Sep 17 00:00:00 2001 From: Pey Lian Lim <2090236+pllim@users.noreply.github.com> Date: Mon, 11 Jan 2021 17:11:52 -0500 Subject: [PATCH 3/3] Minor clean-ups [skip codeql] --- .github/labeler.yml | 4 +--- MANIFEST.in | 2 +- pyproject.toml | 3 +-- setup.cfg | 16 ---------------- setup.py | 6 ++---- synphot/binning.py | 2 +- tox.ini | 3 +++ 7 files changed, 9 insertions(+), 27 deletions(-) diff --git a/.github/labeler.yml b/.github/labeler.yml index 73164fa6..cff4f6ab 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -5,7 +5,6 @@ docs: - docs/_static/* - docs/_templates/* - licenses/* - - synphot/docstrings.py - '*.md' - any: ['*.rst', '!CHANGES.rst'] @@ -27,9 +26,8 @@ binning: - synphot/binning.py C-extension: - - synphot/docstrings.py - - synphot/setup_package.py - synphot/src/* + - synphot/include/* config: - synphot/config.py diff --git a/MANIFEST.in b/MANIFEST.in index 922d52b6..ec44c37e 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -5,8 +5,8 @@ include CITATION.md include setup.cfg include pyproject.toml -recursive-include synphot/include *.h +recursive-include synphot/include *.h recursive-include synphot *.c recursive-include docs * recursive-include licenses * diff --git a/pyproject.toml b/pyproject.toml index 55aee82f..3823812b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,6 +2,5 @@ requires = ["setuptools>=30.3.0", "setuptools_scm", "wheel", - "oldest-supported-numpy" - ] + "oldest-supported-numpy"] build-backend = "setuptools.build_meta" diff --git a/setup.cfg b/setup.cfg index fa20999a..cae549d8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -70,27 +70,11 @@ ignore = I100,I201,W504 source = synphot omit = synphot/_astropy_init* - synphot/cython_version* - synphot/setup_package* - synphot/*/setup_package* - synphot/*/*/setup_package* synphot/tests/* - synphot/*/tests/* - synphot/*/*/tests/* synphot/version* - synphot/extern/* - synphot/docstrings.py */synphot/_astropy_init* - */synphot/cython_version* - */synphot/setup_package* - */synphot/*/setup_package* - */synphot/*/*/setup_package* */synphot/tests/* - */synphot/*/tests/* - */synphot/*/*/tests/* */synphot/version* - */synphot/extern/* - */synphot/docstrings.py [coverage:report] exclude_lines = diff --git a/setup.py b/setup.py index 0465311e..e0cccf95 100755 --- a/setup.py +++ b/setup.py @@ -5,8 +5,6 @@ import sys from setuptools import setup, Extension -LOCALROOT = 'synphot' - def get_extensions(): from collections import defaultdict @@ -15,9 +13,9 @@ def get_extensions(): cfg = defaultdict(list) cfg['include_dirs'].extend([ numpy.get_include(), - os.path.join(LOCALROOT, "include")]) + os.path.join('synphot', 'include')]) cfg['sources'] = [ - os.path.join(LOCALROOT, 'src', 'synphot_utils.c')] + os.path.join('synphot', 'src', 'synphot_utils.c')] cfg = dict((str(key), val) for key, val in cfg.items()) return [Extension('synphot.synphot_utils', **cfg)] diff --git a/synphot/binning.py b/synphot/binning.py index 73179585..cca53130 100644 --- a/synphot/binning.py +++ b/synphot/binning.py @@ -20,7 +20,7 @@ def _slow_calcbinflux(len_binwave, i_beg, i_end, avflux, deltaw): This is only used if ``synphot.synphot_utils`` C-extension import fails. - See docstrings.py + See ``include/synphot_utils.h``. """ binflux = np.empty(shape=(len_binwave, ), dtype=np.float64) diff --git a/tox.ini b/tox.ini index fff90cbf..2453c075 100644 --- a/tox.ini +++ b/tox.ini @@ -12,6 +12,7 @@ isolated_build = true # Pass through the following environemnt variables which are needed for the CI passenv = HOME WINDIR CC CI +# FIXME: Cannot do this, need to force C-ext to build below. # Run the tests in a temporary directory to make sure that we don't import # package from the source tree #changedir = .tmp/{envname} @@ -52,7 +53,9 @@ extras = alldeps: all commands = + # FIXME: Not sure why need this for tox to see the C-ext python setup.py build_ext --inplace + # End of FIXME pip freeze !cov: pytest --pyargs synphot {toxinidir}/docs {posargs} cov: pytest --pyargs synphot {toxinidir}/docs --cov synphot --cov-config={toxinidir}/setup.cfg {posargs}