Repository navigation
pxd files missing in the PyPI package #64
Description
In the package currently available on PyPI (v1.1.4), the two .pxd files (c_numpy.pxd and c_python.pxd) in pyregion/ are missing.
It is thus not possible to generate _region_filter.c directly from _region_filter.pyx using Cython.
Instead, the .c file itself is shipped.
Please ensure that for the next releases, the pxd files are shipped, and not the c file.
Activity
astrofrog commentedon Nov 11, 2015
@ViviCoder - shipping the C code instead of the Cython files in the tarballs is common practice for packages in the Astropy project. We made this decision because otherwise people would need Cython to install the package.
If you are interested in tweaking the Cython files, I recommend you instead work from a clone of the repository.
ViviCoder commentedon Nov 11, 2015
I am responsible for the packaging of pyregion in Debian and the Debian policy is to work from source.
I understand your concern about the users, and to have everybody satisfied, I suggest you ship pxd files AND c files.
Would you agree?
astrofrog commentedon Nov 11, 2015
@olebole - could you comment on this request? We don't do this for astropy core right?
olebole commentedon Nov 11, 2015
For Astropy, I couldn't find
.pxdfiles. However,.pyxfiles are shipped with the tar distribution, and they are cythonized to re-generate the C files during the Debian build. You may check this f.e. for the 1.0.5 i386 build (search for "cythoning"). Other affiliated packages, like astroscrappy also distribute.pxdand.pyxfiles in their tar files (just checked because I am currently working on it :-) ).It should also not cause problems for you to just include these files -- you can leave the generated
.cfiles there as well for convenience. Is there a reason not to ship.pxd/.pyx?spearsem commentedon Mar 14, 2016
Sorry for the late bump on the thread. I just wanted to add that it is important to ship the .pxd files because that is the way someone else can
cimportfrom your library, if they are doing their own thing at the Cython level. It's also much more readable than digging through auto-generated C for declarations. Consider a case when using a Cython fused type ... thecdeffunction will result in many different type-specific variations (overloads) in the generated C source. It doesn't make sense for someone to try to reason about the code from the point of view of the auto-generated parts ... instead better to let them reason about what the developer had explicitly declared.astrofrog commentedon Mar 14, 2016
Ok - I'd be fine with including these files! A pull request to do this would be welcome :)
ViviCoder commentedon Mar 15, 2016
How do you generate your tarballs?
astrofrog commentedon Mar 15, 2016
We normally do
python setup.py sdist- it looks like we should add a line to theMANIFEST.into include Cython files, as done in the core astropy package: https://github.com/astropy/astropy/blob/master/MANIFEST.in#L9ViviCoder commentedon Mar 15, 2016
Done.
cdeil commentedon Aug 1, 2016
I also didn't know that Cython files are included in the tarballs. I checked with Astropy and photutils, and there it's the case.
As commented here
#67 (comment)
the Cython files are now included and will be part of the next release.
Thanks @ViviCoder !