You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have some ideas on the redesign, and want to put them down:
First, I think we should not hard-code flags, and instead allow them to be passed through, like TARGET_LANGUAGE and LANGUAGE_LEVEL unless there's a benefit for us (possibly with TARGET_LANGUAGE? Edit: Yes, we set the extension from this). Users should generally set these options in source files anyway, and if not, they can just as easily pass them through CYTHON_ARGS without us adding lots of custom options. We should also not try to set defaults for the user, and leave that up to Cython.
The command supports adding multiple input files and a single output file - not sure what that does, but if it's useful, we should mimic that here. I don't think there's a lot of benefit in supporting multiple outputs (users can just write a loop) - I'd like to mimic cythonize as closely as possible, just trying to bring it into CMake.
I think instead of an output variable, maybe we should support setting the OUTPUT file directly? That way users can choose where to put the output file location easily. (This also would work best if we don't support multiple input files to multiple output files, above)
Once these are addressed, I think the back-compat "add_cython_target" could be designed using "cython_compile_pyx" Ahh, nice, already done. It could even only exist in scikit-build (classic), as it has a really poor name (it doesn't have anything to do with targets). The "official" version upstreamed to Cython could be called "cythonize".
The text was updated successfully, but these errors were encountered:
Seee cython/cython#5486 (comment) - maybe we should require the LANGUAGE keyword? Currently it changes based on the LANGUAGES set (C unless only CXX is available), which might be slightly supposing? Unless we can detect the language, might be best to just require this for now. Also, I think LANGUAGE is better than TARGET_LANGUAGE, as it's unambiguous already.
I have some ideas on the redesign, and want to put them down:
First, I think we should not hard-code flags, and instead allow them to be passed through, like
TARGET_LANGUAGE
andLANGUAGE_LEVEL
unless there's a benefit for us (possibly withTARGET_LANGUAGE
? Edit: Yes, we set the extension from this). Users should generally set these options in source files anyway, and if not, they can just as easily pass them throughCYTHON_ARGS
without us adding lots of custom options. We should also not try to set defaults for the user, and leave that up to Cython.The command supports adding multiple input files and a single output file - not sure what that does, but if it's useful, we should mimic that here. I don't think there's a lot of benefit in supporting multiple outputs (users can just write a loop) - I'd like to mimic
cythonize
as closely as possible, just trying to bring it into CMake.I think instead of an output variable, maybe we should support setting the
OUTPUT
file directly? That way users can choose where to put the output file location easily. (This also would work best if we don't support multiple input files to multiple output files, above)Once these are addressed,
I think the back-compat "add_cython_target" could be designed using "cython_compile_pyx"Ahh, nice, already done. It could even only exist in scikit-build (classic), as it has a really poor name (it doesn't have anything to do with targets). The "official" version upstreamed to Cython could be called "cythonize".The text was updated successfully, but these errors were encountered: