Repository: cython/cython
Pull request: [Docs] Reorganize and improve documenation of `include_dirs` and `include_path`
This PR:

* Fixes wrong documentation of `include_path` which does not control search path of C/C++ headers - fixes #1480
* Improves documentation of `Extention` parameters used in example to have it self contained and reader does not need to visit setuptools documentation for details.
* Adds *Compilation using setuptools* chapter to *Cython for NumPy users* and include numpy related documentation from *Source files and compilation*.


Problem statement:
docs error: cythonize(include_path=x) does not set C include path
The documentation in [compilation.rst](../blob/master/docs/src/reference/compilation.rst#configuring-the-c-build) claims that

```
cythonize(extensions, include_path=[numpy.get_include()])
```

adds the numpy include path to the include file search path used by the compiler. This has been quoted many times and the documented feature is used by many packages, although with zero effect: `include_path` only adds to the search path used by Cython to find `pyx` and `pxd` files.

Some alternative ways to address this:
1. Change docs to match code by removing the misleading section.
2. Change code to match docs by adding the feature. 
3. Change docs and code to use `cython(include_dirs=[])` as a way for globally specifying C include paths. 
4. Change it all to have `cython_include_dirs` and `c_include_dirs` (or path).

3 has the benefit that code using `include_path` will continue to function as before (no effect on C include path), while making the feature people have obviously been expecting available. The term `include_dir` would then consistently (albeit not obviously) refer to the C includes and the term `include_path` to the Cython includes.

Hints from issue discussion:
Whatever you do, make sure not to break existing packages using `include_path` or `include_dirs` correctly :-)
I'd say it's not a doc bug but a Cython bug. `cythonize` produces an `Extension` object which has `include_dirs` as one of its attributes. `cythonize` should be setting this attribute.

```
>>> from Cython.Build import cythonize
>>> import numpy as np
>>> e=cythonize("test.pyx",include_path=[np.get_include()])
>>> len(e)
1
>>> e[0].include_dirs
[]
```

Here's a patch as a suggestion. 
I could make it pick the possible `Extension` options from the `**options` of `cythonize()` easily enough using the `distutils_settings` list, but I'm not certain that the`cythonize()` parameter namespace would benefit. In particular, `include_path` (referring to Cython includes only) and `include_dir` (referring to C/C++ compiler include directives) are so similar that confusion would be guaranteed.

I was under impression that NumPy's C include path is being passed to `cythonize` because Cython actually needs the header files for code generation (they effectively being included with `cimport`). Well, if you're sure it doesn't...

> Cython actually needs the header files for code generation

Cython doesn't need any `.h` files. It just generates `#incude "foo.h"` statements and it's the C compiler who will actually read the header files.

Cython does look for headers in `include_dirs`, but only to add them as dependenies. It's not an error (not should it be) if the headers are not found.
Having just hit this and spent a while trying to figure out how to work around it, I'd like to voice my support for solution #2 (I can't see the harm, but I don't know much about this, so ...), but also just as an interim measure doing #1 or some variant to fix the docs, which is explicitly claiming something with numpy that doesn't work. I know I'm not the only one to be mislead by this, just by how very often this issue shows up:

https://www.google.com/search?q=numpy%2Farrayobject.h%27+file+not+found&oq=numpy%2Farrayobject.h%27+file+not+found&aqs=chrome..69i57j69i58.427j0j7&sourceid=chrome&ie=UTF-8

For posterity, my interim solution was to pass in my own Extension() to cythonize, which does kinda take away from the benefit of having cythonize in the first place IMHO.

```python
from setuptools import setup, Extension
from Cython.Build import cythonize
import numpy as np

setup(
    name='xyz',
    ext_modules=cythonize(
        Extension(
            "xyz",
            sources=[a/b.pyx"],
            include_dirs=[np.get_include()]
        )
    ),
    install_requires=["numpy"]
)
```
Sorry for the lack of productive criticism, but having a non-working documentation, apparently for several years, is not a great idea for a project.
> Sorry for the lack of productive criticism, but having a non-working documentation, apparently for several years, is not a great idea for a project.

I would not try to keep you from sending a pull request.
My guess is that we named the parameter `include_path` rather than `include_dirs` at the time to distinguish it from the `include_dirs` parameter of `Extension` objects. Or, alternatively, without thinking much. Hard to say now... ;)

I'd be happy to merge #1512 if someone takes the time to fix the documented option names and rebase it.
@scoder: I'll try to find the time this week to have another look at it. It's been a while...

Commit message: Reorganize compilation documentation

Patch to review:
diff --git a/docs/src/userguide/numpy_tutorial.rst b/docs/src/userguide/numpy_tutorial.rst
index d2828ebaba4..b45b1964a4d 100644
--- a/docs/src/userguide/numpy_tutorial.rst
+++ b/docs/src/userguide/numpy_tutorial.rst
@@ -108,8 +108,11 @@ then execute :
 
 This will install the newest Cython into SAGE.
 
+Compilation
+===========
+
 Manual compilation
-====================
+------------------
 
 As it is always important to know what is going on, I'll describe the manual
 method here. First Cython is run:
@@ -143,6 +146,62 @@ in your Cython code.
 This creates :file:`yourmod.so` in the same directory, which is importable by
 Python by using a normal ``import yourmod`` statement.
 
+
+Compilation using setuptools
+----------------------------
+
+Setuptools allows us to create setup.py file to automate both compilation of Cython files and generated C files.::
+
+    from setuptools import Extension, setup
+    from Cython.Build import cythonize
+    import numpy
+
+    extensions = [
+        Extension("*", ["*.pyx"],
+            library_dirs=[numpy.get_include()]),
+    ]
+    setup(
+        name="My hello app",
+        ext_modules=cythonize(extensions),
+    )
+
+Path to NumPy headers is passed to C compiler via ``library_dirs=[numpy.get_include()])`` parameter.
+
+.. note::
+
+    Using memoryviews or importing NumPy with ``import numpy`` does not mean that
+    you have to add the path to NumPy include files. You need to add this path only
+    if you use ``cimport numpy``.
+
+Despite this, you may still get warnings like the following from the compiler,
+because Cython is not disabling the usage of the old deprecated Numpy API::
+
+   .../include/numpy/npy_1_7_deprecated_api.h:15:2: warning: #warning "Using deprecated NumPy API, disable it by " "#defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
+
+In Cython 3.0, you can get rid of this warning by defining the C macro
+``NPY_NO_DEPRECATED_API`` as ``NPY_1_7_API_VERSION``
+in your build, e.g.::
+
+    # distutils: define_macros=NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION
+
+or (see below)::
+
+    Extension(
+        ...,
+        define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")],
+    )
+
+With older Cython releases, setting this macro will fail the C compilation,
+because Cython generates code that uses this deprecated C-API.  However, the
+warning has no negative effects even in recent NumPy versions including 1.18.x.
+You can ignore it until you (or your library's users) switch to a newer NumPy
+version that removes this long deprecated API, in which case you also need to
+use Cython 3.0 or later.  Thus, the earlier you switch to Cython 3.0, the
+better for your users.
+
+
+
+
 The first Cython program
 ==========================
 
diff --git a/docs/src/userguide/source_files_and_compilation.rst b/docs/src/userguide/source_files_and_compilation.rst
index d08d430ec0f..f7dd8bf3afc 100644
--- a/docs/src/userguide/source_files_and_compilation.rst
+++ b/docs/src/userguide/source_files_and_compilation.rst
@@ -150,7 +150,7 @@ documentation`_. To compile the extension for use in the current directory use:
 Configuring the C-Build
 ------------------------
 
-If you have include files in non-standard places you can pass an
+If you have Cython include files in non-standard places you can pass an
 ``include_path`` parameter to ``cythonize``::
 
     from setuptools import setup
@@ -161,43 +161,6 @@ If you have include files in non-standard places you can pass an
         ext_modules=cythonize("src/*.pyx", include_path=[...]),
     )
 
-Often, Python packages that offer a C-level API provide a way to find
-the necessary include files, e.g. for NumPy::
-
-    include_path = [numpy.get_include()]
-
-.. note::
-
-    Using memoryviews or importing NumPy with ``import numpy`` does not mean that
-    you have to add the path to NumPy include files. You need to add this path only
-    if you use ``cimport numpy``.
-
-Despite this, you may still get warnings like the following from the compiler,
-because Cython is not disabling the usage of the old deprecated Numpy API::
-
-   .../include/numpy/npy_1_7_deprecated_api.h:15:2: warning: #warning "Using deprecated NumPy API, disable it by " "#defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
-
-In Cython 3.0, you can get rid of this warning by defining the C macro
-``NPY_NO_DEPRECATED_API`` as ``NPY_1_7_API_VERSION``
-in your build, e.g.::
-
-    # distutils: define_macros=NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION
-
-or (see below)::
-
-    Extension(
-        ...,
-        define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")],
-    )
-
-With older Cython releases, setting this macro will fail the C compilation,
-because Cython generates code that uses this deprecated C-API.  However, the
-warning has no negative effects even in recent NumPy versions including 1.18.x.
-You can ignore it until you (or your library's users) switch to a newer NumPy
-version that removes this long deprecated API, in which case you also need to
-use Cython 3.0 or later.  Thus, the earlier you switch to Cython 3.0, the
-better for your users.
-
 If you need to specify compiler options, libraries to link with or other
 linker options you will need to create ``Extension`` instances manually
 (note that glob syntax can still be used to specify multiple extensions
@@ -222,9 +185,30 @@ in one line)::
         ext_modules=cythonize(extensions),
     )
 
+Some useful options to know about are
+
+* ``include_dirs``- list of directories to search for C/C++ header files (in Unix form for portability)
+* ``libraries`` - list of library names (not filenames or paths) to link against,
+* ``library_dirs`` - list of directories to search for C/C++ libraries at link time.
+
 Note that when using setuptools, you should import it before Cython, otherwise,
 both might disagree about the class to use here.
 
+Often, Python packages that offer a C-level API provide a way to find
+the necessary C header files::
+
+    from setuptools import Extension, setup
+    from Cython.Build import cythonize
+
+    extensions = [
+        Extension("*", ["*.pyx"],
+            include_dirs=["/usr/local/include"]),
+    ]
+    setup(
+        name="My hello app",
+        ext_modules=cythonize(extensions),
+    )
+
 If your options are static (for example you do not need to call a tool like
 ``pkg-config`` to determine them) you can also provide them directly in your
 .pyx or .pxd source file using a special comment block at the start of the file::
@@ -258,9 +242,7 @@ as follows::
     )
 
 The :class:`Extension` class takes many options, and a fuller explanation can
-be found in the `setuptools documentation`_. Some useful options to know about
-are ``include_dirs``, ``libraries``, and ``library_dirs`` which specify where
-to find the ``.h`` and library files when linking to external libraries.
+be found in the `setuptools documentation`_.
 
 .. _setuptools documentation: https://setuptools.readthedocs.io/
 


Code review comments:
diff_hunk: @@ -143,6 +146,62 @@ in your Cython code.
 This creates :file:`yourmod.so` in the same directory, which is importable by
 Python by using a normal ``import yourmod`` statement.
 
+
+Compilation using setuptools
+----------------------------
+
+Setuptools allows us to create setup.py file to automate both compilation of Cython files and generated C files.::
+
+    from setuptools import Extension, setup
+    from Cython.Build import cythonize
+    import numpy
+
+    extensions = [
+        Extension("*", ["*.pyx"],
+            library_dirs=[numpy.get_include()]),
+    ]
+    setup(
+        name="My hello app",
+        ext_modules=cythonize(extensions),
+    )
+
+Path to NumPy headers is passed to C compiler via ``library_dirs=[numpy.get_include()])`` parameter.
+
+.. note::
+
+    Using memoryviews or importing NumPy with ``import numpy`` does not mean that
+    you have to add the path to NumPy include files. You need to add this path only
+    if you use ``cimport numpy``.
+
+Despite this, you may still get warnings like the following from the compiler,
+because Cython is not disabling the usage of the old deprecated Numpy API::
+
+   .../include/numpy/npy_1_7_deprecated_api.h:15:2: warning: #warning "Using deprecated NumPy API, disable it by " "#defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
+
+In Cython 3.0, you can get rid of this warning by defining the C macro
+``NPY_NO_DEPRECATED_API`` as ``NPY_1_7_API_VERSION``
+in your build, e.g.::
+
+    # distutils: define_macros=NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION
+
+or (see below)::
+
+    Extension(
+        ...,
+        define_macros=[("NPY_NO_DEPRECATED_API", "NPY_1_7_API_VERSION")],
+    )
+
+With older Cython releases, setting this macro will fail the C compilation,
+because Cython generates code that uses this deprecated C-API.  However, the
+warning has no negative effects even in recent NumPy versions including 1.18.x.
path: docs/src/userguide/numpy_tutorial.rst
text: @user1:
```suggestion
warning has no negative effects even in recent NumPy versions.
```

Just going to suggest getting rid of the version, partly because the sentence is a bit awkward, and partly because it's not been a recent version for a while

diff_hunk: @@ -143,6 +146,62 @@ in your Cython code.
 This creates :file:`yourmod.so` in the same directory, which is importable by
 Python by using a normal ``import yourmod`` statement.
 
+
+Compilation using setuptools
+----------------------------
+
+Setuptools allows us to create setup.py file to automate both compilation of Cython files and generated C files.::
+
+    from setuptools import Extension, setup
+    from Cython.Build import cythonize
+    import numpy
+
+    extensions = [
+        Extension("*", ["*.pyx"],
+            library_dirs=[numpy.get_include()]),
+    ]
+    setup(
+        name="My hello app",
+        ext_modules=cythonize(extensions),
+    )
+
+Path to NumPy headers is passed to C compiler via ``library_dirs=[numpy.get_include()])`` parameter.
path: docs/src/userguide/numpy_tutorial.rst
text: @user1:
```suggestion
The path to the NumPy headers is passed to the C compiler via the ``library_dirs=[numpy.get_include()])`` parameter.
```

@user1:
Also is this definitely `library_dirs`?

diff_hunk: @@ -143,6 +146,62 @@ in your Cython code.
 This creates :file:`yourmod.so` in the same directory, which is importable by
 Python by using a normal ``import yourmod`` statement.
 
+
+Compilation using setuptools
+----------------------------
+
+Setuptools allows us to create setup.py file to automate both compilation of Cython files and generated C files.::
+
+    from setuptools import Extension, setup
+    from Cython.Build import cythonize
+    import numpy
+
+    extensions = [
+        Extension("*", ["*.pyx"],
+            library_dirs=[numpy.get_include()]),
path: docs/src/userguide/numpy_tutorial.rst
text: @user1:
`library_dirs`?

@author:
Of course not. I forgot to push the commit fixing it :-/