feat: rewrite

[henryiii]

Close #18.

• Remove the back-compat method for now; I think it could exist only inside scikit-build (classic)
• Currently require 3.20 (for cmake_path), which was implicitly required before. Reduced to at least 3.15, probably 3.7, but DEPFILE only works on Ninja until Make was added in 3.20.
• Try to support spaces in paths correctly
• Add a test using scikit-build-core
• Add a Cython::Cython target
• Try to compute the language automatically if not specified and both C and CXX are enabled.

Along with this, this is the new syntax for the Cython_compile_pyx function:

Cython_compile_pyx(<pyx_file>
                  [LANGUAGE C | CXX]
                  [CYTHON_ARGS <args> ...]
                  [OUTPUT <OutputFile>]
                  [OUTPUT_VARIABLE <OutputVariable>]
                  )

OUTPUT sets the file path, otherwise it's the same name with new extension in the current binary dir. OUTPUT_VARIABLE contains the file path to the generated output.

• Add a cached CYTHON_ARGS variable to set the default. It would be useful for adding things like annotation on all calls.
• Why would you pass multiple files to cythonize? It only produces one output. Should we allow multiple files here, and just treat the first one as special? Edit: leaving this off for now. It could be added, it would be incompatible with OUTPUT and a list of files would be placed in OUTPUT_VARIABLE.

If this works well, we can propose a cythonize function upstream to Cython; if they make changes to the signature, we can wrap those once it's released and provide "cythonize" for previous versions of Cython.

[henryiii]

@vyasr, thoughts?

[vyasr]

What is the back-compat method that you meant? If you mean the add_cython_target, yes I agree that should go. I read through #18 and I think as you noted that some of the ideas were already implemented in my original prototype, but also not all of them made it from my prototype into what's in this repo so there are probably additional improvements still to be made here. This was definitely one of the changes I had intended.

I don't have any powers in this repo unfortunately, but generally the changes look good to me.

[vyasr]

Should we stop documenting the CYTHON_EXECUTABLE variable altogether and just say that the only supported way to get it is via this target?

[henryiii]

They are different when cross compiling. Targets will use the cross-compiling emulator, while variables are just variables. Also, target's can't be used in some places like during the configure process, while variables can.

[vyasr]

Good point, I guess the cython command could itself have been cythonized and therefore be native to the target system.

[vyasr]

We added depfile support in https://github.com/rapidsai/rapids-cmake/pull/579/files, so you may want to double-check there to see if we did anything differently from what you did here (good or bad) since both our code and the new code in this repo are very stripped down relative to what UseCython.cmake looked like before this PR.

[jcfr]

I believe all features have been added while maintaining the history and giving credits to the various contributors.

For reference, depfile support has indeed been added through 490f199

[henryiii]

The only thing I saw I commented on that PR; that one is missing the depfile as output, while I think it's correct to list it there (since it's not always output, just usually, so CMake likely can't assume it's output).

[jcfr]

For reference, testing of the module using scikit-build-core is being added through:

• Add core cython hello scikit-build-sample-projects#43

[henryiii]

Added a few more fixes; you can now use UseCython directly if you want. I've also made an F2Py version at https://github.com/scikit-build/f2py-cmake.

[vyasr]

Apologies, totally dropped the ball on this one. I'll take a pass at reviewing, but overall it seems like it's come a long way and is a definite improvement.

[henryiii]

I think most of this is in, minus some stylistic stuff