Conversation
neutrinoceros
left a comment
neutrinoceros
left a comment
There was a problem hiding this comment.
Thanks for the ping. I don't understand how this is supposed to work yet, and with a red CI I'm not yet convinced it should.
setup.py
Outdated
| 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: |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I copied this from netcdf4-python - I think it can be removed.
|
Don't know why the wheels fail some tests |
I can reproduce that failure locally and I get significantly different https://gist.github.com/ocefpaf/086f5540ca9705dc111509b02b282801 |
| include_dirs=[numpy.get_include(),])] | ||
| for e in ext_modules: | ||
| e.cython_directives = {'language_level': "3"} | ||
| e.compiler_directives = COMPILER_DIRECTIVES |
There was a problem hiding this comment.
Looks like this part is being ignored. When I add #cython: c_api_binop_methods=True to the top of the pyx everything works as expected.
3 participants
@neutrinoceros hopefully this will help with PR #372