[cython/cython] DOC: recommend not distributing generated code (PR #6201)
Clément Robert
This is a tentative fix for #5089, following @scoder's reply
I feel that the section as it exists now still has value as an historical document, and the modern recommendation is simple enough that it can maybe live alongside it instead of replacing it, but I'm happy to change my approach if desired.
You can view, comment on, or merge this pull request online at:
https://github.com/cython/cython/pull/6201
Commit Summary
- 29df352 DOC: recommend not distributing generated code
File Changes
(1 file)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
da-woods
Thank for making a start on this.
My personal view is that we shouldn't keep outdated advice around for historical reasons, and to try to keep the documentation fairly simple. I think it's worth mentioning as another option for the problem though, but fairly briefly.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
scoder
@scoder commented on this pull request.
In docs/src/userguide/source_files_and_compilation.rst:
> @@ -338,6 +338,15 @@ them through :func:`cythonize`:: Distributing Cython modules ---------------------------- +.. note:: + + This section is historical and contains recommendations that outlived their + purpose. Following recent improvements in the distribution toolchain, it is + now recommended to *not* include generated files in distributions, + and require Cython at build-time as defined in + `PEP 518 <https://www.python.org/dev/peps/pep-0518/>`_ + and `PEP 621 <https://www.python.org/dev/peps/pep-0621/>`_. + It is strongly recommended that you distribute the generated ``.c`` files as well
I think we can keep much of this section, just with a more optional wording.
⬇️ Suggested change-It is strongly recommended that you distribute the generated ``.c`` files as well +It is possible to distribute the generated ``.c`` files as well
etc.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Clément Robert
@neutrinoceros pushed 1 commit.
- 6e265c5 rephrase recommendations as supported options
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Clément Robert
Thank you both for your input. I went with @scoder's suggestion to rephrase the section for now, simply because it was closer to what I personally had in mind originally. If the section is considered harmful in the future it can still be removed at a later point. That said, I'm still happy to revisit if the emerging consensus differs.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
scoder
> +.. note:: + + This section is historical and contains recommendations that outlived their + purpose. Following recent improvements in the distribution toolchain, it is + now recommended to *not* include generated files in distributions, + and require Cython at build-time as defined in + `PEP 518 <https://www.python.org/dev/peps/pep-0518/>`_ + and `PEP 621 <https://www.python.org/dev/peps/pep-0621/>`_. + See :ref:`basic_setup.py`.
I think we can now also make this paragraph a normal part of the text:
⬇️ Suggested change-.. note:: - - This section is historical and contains recommendations that outlived their - purpose. Following recent improvements in the distribution toolchain, it is - now recommended to *not* include generated files in distributions, - and require Cython at build-time as defined in - `PEP 518 <https://www.python.org/dev/peps/pep-0518/>`_ - and `PEP 621 <https://www.python.org/dev/peps/pep-0621/>`_. - See :ref:`basic_setup.py`. +Following recent improvements in the distribution toolchain, it is +not recommended to include generated files in source distributions. +Instead, `require` Cython at build-time to generate the C/C++ files, +as defined in `PEP 518 <https://www.python.org/dev/peps/pep-0518/>`_ +and `PEP 621 <https://www.python.org/dev/peps/pep-0621/>`_. +See :ref:`basic_setup.py`.
In docs/src/userguide/source_files_and_compilation.rst:
> +It is possible to distribute the generated ``.c`` files as well as your Cython sources, so that users can install your module without needing⬇️ Suggested change
-It is possible to distribute the generated ``.c`` files as well -as your Cython sources, so that users can install your module without needing +It is, however, possible to distribute the generated ``.c`` files together with +your Cython sources, so that users can install your module without needing
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Clément Robert
@neutrinoceros pushed 1 commit.
- 9bd58cf Apply suggestions from code review
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
scoder
Merged #6201 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
scoder
Thanks
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Clément Robert
Thank you ! should #5089 be closed ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.