-
Notifications
You must be signed in to change notification settings - Fork 76
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
Reimplement managed raster class in C++ #1597
Changes from all commits
5e58f60
7c51c08
a55d57c
068d886
8576088
7f2ec80
11ef40f
c7486f2
048e769
cccf0e6
8d104a4
6ca0f78
61d1f65
d3aebac
eb15fa1
93ec4b9
4506414
1ff72b6
6acaea6
9316d20
0d4805a
53b15d0
06c8648
1749eb9
e4c234f
bbd7c5a
7bc4b58
efc5527
474a89a
a63532f
05aa968
81af8a3
a0e5810
d5bd998
09a6f8e
c669180
61883f5
d236af1
7e12302
c664e43
baac039
8cf1e6b
5932fdb
8fe87ec
d7d3b09
4e5e717
5f6a647
8defb6f
6245b6a
364d7ff
386beb2
7b994e2
f62a696
8109a60
da23f3e
31565e8
8c31a12
b88567c
36c26b2
fc8dd38
e6a370e
2cf8f5d
ba37a0a
b957881
989a7b6
b8e710f
b532001
9421e2c
9981569
6d556e2
c7742e8
7b9db1f
9071362
ba9cebe
13cc0dc
7a767f5
5b8c04e
947900c
f737225
3a623eb
420145f
dde78df
11cc0ef
25c954a
d50a2c9
3adae2d
24d1028
b3d2a55
45ec188
9b95796
3c9b795
07059b0
041cd1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,14 +15,25 @@ | |
if (not req.startswith(('#', 'hg+', 'git+')) | ||
and len(req.strip()) > 0)] | ||
|
||
# Since OSX Mavericks, the stdlib has been renamed. So if we're on OSX, we | ||
# need to be sure to define which standard c++ library to use. I don't have | ||
# access to a pre-Mavericks mac, so hopefully this won't break on someone's | ||
# older system. Tested and it works on Mac OSX Catalina. | ||
compiler_and_linker_args = [] | ||
if platform.system() == 'Darwin': | ||
compiler_and_linker_args = ['-stdlib=libc++'] | ||
|
||
include_dirs = [numpy.get_include(), 'src/natcap/invest/managed_raster'] | ||
if platform.system() == 'Windows': | ||
compiler_args = ['/std:c++20'] | ||
if 'NATCAP_INVEST_GDAL_LIB_PATH' not in os.environ: | ||
raise RuntimeError( | ||
'env variable NATCAP_INVEST_GDAL_LIB_PATH is not defined. ' | ||
'This env variable is required when building on Windows. If ' | ||
'using conda to manage your gdal installation, you may set ' | ||
'NATCAP_INVEST_GDAL_LIB_PATH="$CONDA_PREFIX/Library".') | ||
library_dirs = [f'{os.environ["NATCAP_INVEST_GDAL_LIB_PATH"]}/lib'] | ||
include_dirs.append(f'{os.environ["NATCAP_INVEST_GDAL_LIB_PATH"]}/include') | ||
else: | ||
library_dirs = [] | ||
compiler_args = [] | ||
compiler_and_linker_args = ['-std=c++20'] | ||
library_dirs = [subprocess.run( | ||
['gdal-config', '--libs'], capture_output=True, text=True | ||
).stdout.split()[0][2:]] # get the first argument which is the library path | ||
|
||
class build_py(_build_py): | ||
"""Command to compile translation message catalogs before building.""" | ||
|
@@ -50,13 +61,14 @@ def run(self): | |
Extension( | ||
name=f'natcap.invest.{package}.{module}', | ||
sources=[f'src/natcap/invest/{package}/{module}.pyx'], | ||
include_dirs=[numpy.get_include()] + ['src/natcap/invest/managed_raster'], | ||
extra_compile_args=compiler_args + compiler_and_linker_args, | ||
include_dirs=include_dirs, | ||
extra_compile_args=compiler_args + package_compiler_args + compiler_and_linker_args, | ||
extra_link_args=compiler_and_linker_args, | ||
language='c++', | ||
libraries=['gdal'], | ||
library_dirs=library_dirs, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe GDAL also has a self-configuration utility that might be able to help us find the location of its include directory: https://gdal.org/en/latest/programs/gdal-config.html I don't know that this will help us out on Windows, but it might at least be nicer on linux and mac. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh neat! Thanks for pointing that out. I'm not sure what the solution would be for Windows, to avoid relying on conda - possibly we'd need to make the GDAL lib location configurable as an env variable |
||
define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")] | ||
) for package, module, compiler_args in [ | ||
('managed_raster', 'managed_raster', []), | ||
) for package, module, package_compiler_args in [ | ||
('delineateit', 'delineateit_core', []), | ||
('recreation', 'out_of_core_quadtree', []), | ||
# clang-14 defaults to -ffp-contract=on, which causes the | ||
|
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.
I see we're repeating this new environment variable in a couple places, so would it interfere with anything to move it to the
env
block so we only need to define it once? I'm pretty sure we can to a top-levelenv:
mapping to allow the env var to propagate across all of the different sections.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.
Nope, I see now that we won't know what
$CONDA_PREFIX
is going to be until after we're in the activated environment, so this seems like the most direct way to define it. 👍