[Cython] Static checker for cython extension dependencies?

Fri Jun 10 20:00:09 EDT 2016

https://bitbucket.org/yt_analysis/yt/pull-requests/2225/use-cythonize-to-manage-pxd-dependencies/diff

On Fri, Jun 10, 2016 at 3:45 PM, Robert Bradshaw <robertwb at gmail.com> wrote:
> On Fri, Jun 10, 2016 at 3:21 PM, Nathan Goldbaum <nathan12343 at gmail.com> wrote:
>>
>> On Fri, Jun 10, 2016 at 5:04 PM, Robert Bradshaw <robertwb at gmail.com> wrote:
>>>
>>> On Fri, Jun 10, 2016 at 1:18 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>>> wrote:
>>> > Hmm, so I've looked into this a bit further, and it looks like the
>>> > metadata
>>> > isn't going to be useful for us. Many of our extensions can't be
>>> > autogenerated by cython because they have non-trivial dependencies
>>>
>>> meaning? In my cursory glance, these all look fine, but perhaps you
>>> could point me at a case that isn't very nice. Also, cythonize isn't
>>> necessarily all-or-nothing--you can always manually add Extensions to
>>> your list.
>>>
>>>
>>> > or depend directly on C code.
>>>
>>> cythonize handles dependencies on C code just fine
>>>
>>> > For these extensions, cythonize must be passed an
>>> > instantiated Extension object, and the metadata embedded in the compiled
>>> > C
>>> > file just uses whatever metadata is attached to the Extension object.
>>>
>>> The embedded metadata is the union of what was in the Extension object
>>> and the extra dependencies it found.
>>
>>
>> Hmm, maybe I made a mistake, but when I passed an Extension object to
>> cythonize earlier, I found that the metadata that got written out to the
>> autogenerated C only included the dependencies that were listed in the
>> Extension object's declaration, not additional ones inferred by cython.
>> Perhaps this is a bug?
>
> Sounds like a bug to me.
>
>>> > I think my original idea - to parse pyx files for cimports and add a
>>> > test -
>>> > should allow us to manage this more safely in the future while still
>>> > maintaining fine-grained control over dependencies of C extensions in
>>> > our
>>> > setup.py file.
>>> >
>>> > That said, I'd love to hear alternate ideas.
>>>
>>> "Parse pyx files for cimports" (recursively, handing includes and cdef
>>> extern from ...) is essentially re-implementing cythonize. What is the
>>> advantage of maintaining them by hand? Cythonize was invented because
>>> people are really bad at maintaining these lists manually...and they
>>> can be entirely deduced.
>>
>> The advantage here is that in my couple hours of playing with using
>> cythonize for our setup.py script, I couldn't get it to correctly generate
>> the metadata I needed to validate the dependencies, so it wasn't providing
>> any added value. Again, it's entirely possible I was making a mistake
>> somewhere.
>
> :(. I'll take a quick stab at it.
>
>> Also, separately, the fake_cythonize script you suggested will error out if
>> cython isn't installed and the cython-generated C files do not exist yet.
>
> The fake_cythonize script (and metadata) is intended for the case that
> the user doesn't have Cython, but the C files are shipped. Sounds like
> you require Cython (installing it for the user if they don't have it).
>
>> That certainly makes sense if you expect developers to have cython
>> available, but it's a downgrade with respect to our current setup. Right now
>> we have it set up so that setuptools will install cython into the build
>> environment while it is processing C extensions if it is not already
>> installed. This means if new developers or users come along and want to
>> build yt from source, they won't need to explicitly install cython first -
>> setuptools will take care of it for us. This makes our installation
>> instructions simpler and causes fewer head-scratching crashes when people
>> try to compile yt from source.
>
> Makes sense. In this case, you would have to avoid importing and
> calling cythonize until after your setup.py invoked setup.
>
>>> > On Fri, Jun 10, 2016 at 1:49 PM, Robert Bradshaw <robertwb at gmail.com>
>>> > wrote:
>>> >>
>>> >> We write metadata in the generated C files for just this reason. You
>>> >> can fall back to something like
>>> >> https://gist.github.com/robertwb/25ab9838cc2b9b21eed646834cf4a108 if
>>> >> cython is not available.
>>> >>
>>> >>
>>> >> On Fri, Jun 10, 2016 at 10:55 AM, Nathan Goldbaum
>>> >> <nathan12343 at gmail.com>
>>> >> wrote:
>>> >> > The reason we haven't done that is we would like our setup.py script
>>> >> > to
>>> >> > be
>>> >> > runnable without cython being installed. I think cythonize is being
>>> >> > invoked
>>> >> > (or something similar to it) by setuptools, using a feature added in
>>> >> > setuptools 18.0:
>>> >> > https://setuptools.readthedocs.io/en/latest/history.html#id60
>>> >> >
>>> >> > Is there a way to use cythonize for this build workflow without
>>> >> > importing it
>>> >> > at the top-level in our setup.py file?
>>> >> >
>>> >> > FWIW, our setup.py file is here:
>>> >> >
>>> >> >
>>> >> > https://bitbucket.org/yt_analysis/yt/src/yt/setup.py?at=yt&fileviewer=file-view-default
>>> >> >
>>> >> > On Fri, Jun 10, 2016 at 12:49 PM, Robert Bradshaw
>>> >> > <robertwb at gmail.com>
>>> >> > wrote:
>>> >> >>
>>> >> >> You should be using cythonize rather than listing and maintaining
>>> >> >> the
>>> >> >> Extension definitions themselves.
>>> >> >>
>>> >> >>
>>> >> >>
>>> >> >>
>>> >> >> http://docs.cython.org/src/quickstart/build.html#building-a-cython-module-using-distutils
>>> >> >>
>>> >> >>
>>> >> >> https://github.com/cython/cython/wiki/enhancements-distutils_preprocessing
>>> >> >>
>>> >> >> On Fri, Jun 10, 2016 at 9:18 AM, Nathan Goldbaum
>>> >> >> <nathan12343 at gmail.com>
>>> >> >> wrote:
>>> >> >> > Hi all,
>>> >> >> >
>>> >> >> > I'm working on a pretty large python/cython codebase (yt) that has
>>> >> >> > many
>>> >> >> > interdependent C extensions written in cython.
>>> >> >> >
>>> >> >> > I've found it to be pretty hit or miss to depend on contributors
>>> >> >> > to
>>> >> >> > manually
>>> >> >> > update cython dependency information in our setup.py file. The
>>> >> >> > dependency
>>> >> >> > information seems to only be used by setuptools to trigger
>>> >> >> > recompilation,
>>> >> >> > but critically setuptools will happily compile a C extension for
>>> >> >> > the
>>> >> >> > first
>>> >> >> > time if the depends information is incomplete. This means that
>>> >> >> > during
>>> >> >> > development, if I update a cython routine, there's no way to
>>> >> >> > ensure
>>> >> >> > that
>>> >> >> > other cython routines that cimport the one I modified will be
>>> >> >> > recompiled
>>> >> >> > unless I manually ensure the depends information is updated
>>> >> >> > whenever
>>> >> >> > cython
>>> >> >> > code gains or loses a cimport.
>>> >> >> >
>>> >> >> > To make that more concrete, here's a pull request I just made to
>>> >> >> > yt
>>> >> >> > that
>>> >> >> > adds missing dependencies for a cython header. Without this pull
>>> >> >> > request,
>>> >> >> > setuptools fails to recompile these routines when
>>> >> >> > selection_routines.pxd
>>> >> >> > changes, causing a build failure.
>>> >> >> >
>>> >> >> > https://bitbucket.org/yt_analysis/yt/pull-requests/2220
>>> >> >> >
>>> >> >> > I think it should be possible to write a test for this by defining
>>> >> >> > the
>>> >> >> > dependency information outside of setup.py and parsing grep and
>>> >> >> > looking
>>> >> >> > for
>>> >> >> > all cython files that cimport other cython files defined inside
>>> >> >> > yt.
>>> >> >> > However,
>>> >> >> > before I do that, I'm curious whether anyone has done something
>>> >> >> > similar,
>>> >> >> > or
>>> >> >> > if there is some other way of forcing the dependency information
>>> >> >> > to
>>> >> >> > be
>>> >> >> > complete on the first compilation, rather than just for subsequent
>>> >> >> > incremental recompilations during development.
>>> >> >> >
>>> >> >> > Thanks for your help!
>>> >> >> >
>>> >> >> > -Nathan
>>> >> >> >
>>> >> >> > _______________________________________________
>>> >> >> > cython-devel mailing list
>>> >> >> > cython-devel at python.org
>>> >> >> > https://mail.python.org/mailman/listinfo/cython-devel
>>> >> >> >
>>> >> >> _______________________________________________
>>> >> >> cython-devel mailing list
>>> >> >> cython-devel at python.org
>>> >> >> https://mail.python.org/mailman/listinfo/cython-devel
>>> >> >
>>> >> >
>>> >> >
>>> >> > _______________________________________________
>>> >> > cython-devel mailing list
>>> >> > cython-devel at python.org
>>> >> > https://mail.python.org/mailman/listinfo/cython-devel
>>> >> >
>>> >> _______________________________________________
>>> >> cython-devel mailing list
>>> >> cython-devel at python.org
>>> >> https://mail.python.org/mailman/listinfo/cython-devel
>>> >
>>> >
>>> >
>>> > _______________________________________________
>>> > cython-devel mailing list
>>> > cython-devel at python.org
>>> > https://mail.python.org/mailman/listinfo/cython-devel
>>> >
>>> _______________________________________________
>>> cython-devel mailing list
>>> cython-devel at python.org
>>> https://mail.python.org/mailman/listinfo/cython-devel
>>
>>
>>
>> _______________________________________________
>> cython-devel mailing list
>> cython-devel at python.org
>> https://mail.python.org/mailman/listinfo/cython-devel
>>