Cython not specified as a dependency but implicitly required

[filip-komarzyniec]

I'd like to ask what are the reasons behind the change to the setup.py introduced in one of the latest commits: a289197#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7

This change requires having Cython preinstalled in the environment while building jenkspy (e.g. during pip installing src distribution).

Having one's own library dependant on jenkspy, one has to now specify Cython as his own dependency or have it somehow already installed in the environment (usually by a workaround and dirty hacks to one's CI-workflows). Either way is not desired.

Why cannot Cython be specified as a dependency / build-only dependency to the jenkspy project? Or why the previous approach of try...except block cannot be preserved?

[mthh]

Thanks for your feedback.

The main reason behind the change you point out was simply to no longer track (in Git) the C code generated by Cython and to let it be generated at compile time, which I thought was more practical and would allow the generated code to keep track of any improvements of Cython (but maybe I'm forgetting another reason that led me to do this).

However, you're right, Cython should now be a (build-only) dependency, which I haven't done (this was unintentional).

Is it OK for you if I do that?
Or is it more convient for you if I go back to the previous way handling this aspect? (In fact the try...except block could have been preserved, I had mainly chosen to simplify the setup.py but I'm OK to find a solution that's convenient for you)

[filip-komarzyniec]

Thank you for such a detailed and kind answer, I really do appreciate it!

I get now the reasoning behind the change. From my POV the main goal is to release Cython dependency from my side. Having that in mind either proposed way is ok and acceptable.

If I could suggest an approach -- I'd add Cython to the build-system.requires table in pyproject.toml file. I'm aware that addition of pyproject.toml might require some additional refactoring of the current setup scripts that's why I'm happy to provide a PR for that :)

[mthh]

I'd add Cython to the build-system.requires table in pyproject.toml file.

Yes, it's a great idea.

Thank you for your offer to contribute! So I'll let you make a PR for these changes :)

[filip-komarzyniec]

Hi,

I have a PR ready (on a newly created local branch) but I seem to lack access to push to the repository.

I'll be happy to issue a PR for review as soon as I have access. 🧑🏽‍💻

I guess I'll open a PR from a fork

[mthh]

I guess I'll open a PR from a fork

This is indeed how it's usually done here 👍