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 <rober...@gmail.com> wrote: > On Fri, Jun 10, 2016 at 3:21 PM, Nathan Goldbaum <nathan12...@gmail.com> > wrote: >> >> On Fri, Jun 10, 2016 at 5:04 PM, Robert Bradshaw <rober...@gmail.com> wrote: >>> >>> On Fri, Jun 10, 2016 at 1:18 PM, Nathan Goldbaum <nathan12...@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 <rober...@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 >>> >> <nathan12...@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 >>> >> > <rober...@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 >>> >> >> <nathan12...@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@python.org >>> >> >> > https://mail.python.org/mailman/listinfo/cython-devel >>> >> >> > >>> >> >> _______________________________________________ >>> >> >> cython-devel mailing list >>> >> >> cython-devel@python.org >>> >> >> https://mail.python.org/mailman/listinfo/cython-devel >>> >> > >>> >> > >>> >> > >>> >> > _______________________________________________ >>> >> > cython-devel mailing list >>> >> > cython-devel@python.org >>> >> > https://mail.python.org/mailman/listinfo/cython-devel >>> >> > >>> >> _______________________________________________ >>> >> cython-devel mailing list >>> >> cython-devel@python.org >>> >> https://mail.python.org/mailman/listinfo/cython-devel >>> > >>> > >>> > >>> > _______________________________________________ >>> > cython-devel mailing list >>> > cython-devel@python.org >>> > https://mail.python.org/mailman/listinfo/cython-devel >>> > >>> _______________________________________________ >>> cython-devel mailing list >>> cython-devel@python.org >>> https://mail.python.org/mailman/listinfo/cython-devel >> >> >> >> _______________________________________________ >> cython-devel mailing list >> cython-devel@python.org >> https://mail.python.org/mailman/listinfo/cython-devel >> _______________________________________________ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
