[Cython] star-imports in Cython/Includes/cpython considered harmful

Robert Bradshaw robertwb at math.washington.edu
Tue Feb 21 21:14:19 CET 2012


On Mon, Feb 20, 2012 at 12:45 AM, Stefan Behnel <stefan_ml at behnel.de> wrote:
> Hi,
>
> I just noticed that the star-imports in the cpython package have serious
> side effects. The cpython/__init__.pxd file has this in it:
>
> """
> from cpython.version cimport *
> from cpython.ref cimport *
> from cpython.exc cimport *
> from cpython.module cimport *
> from cpython.mem cimport *
> ...
> """
>
> This means that a direct cimport of any of those recursively cimported
> names from the cpython package namespace will always trigger a complete
> cimport of all cpython.* modules, thus
>
> 1) wasting compile time

Which is still pretty cheap (and should be made extremely cheap for
unchanging pxd files).

> 2) polluting the cpython package namespace

It's neither inspected (by the end user) at runtime, nor in danger of
conflicts, so I don't see this as a big deal. "Flat is better than
nested."

> 3) polluting the internal state of Cython's list of cimported modules

This is really (1), other than that it doesn't really hurt to have
extra keys in a dict.

> 4) leading to things being defined in the generated C code that don't need
> to be there, specifically the object structs of PyBoolObject and
> PyComplexObject, which we provide definitions for in cpython.bool and
> cpython.complex.

This is an actual issue that should be fixed, and avoiding emitting
unused code is a worthwhile goal in many contexts.

> The point where I noticed this is when I got test errors in PyPy because it
> doesn't expose the bool/complex structs. That wasn't because the test
> actually used them, it was purely because it had this cimport in it:
>
>  from cpython cimport PyObject
>
> If it had used this instead:
>
>  from cpython.object cimport PyObject
>
> Cython would still have parsed all of those .pxd files, but at least it
> wouldn't have used those declarations to do any harm.
>
> Now, I'm 100% certain that there is user code out there that does this as
> well. Fixing this bug by removing all the star-imports will break that
> code, but keeping the cimports in the package will trap the average lazy
> user into cimporting stuff from there directly.
>
> I'll add a big warning to the package for now, but I wonder if we shouldn't
> accept breaking this code at some point (and sooner is always better than
> later).

I actually don't think it's a bad thing to allow people to be "lazy"
and import directly from cpython rather than have to hunt down the
specific subpackages that things live in. In fact, it's cleaner in
some sense to not have to know about the subdivisions of the cpython
library. (It's like including "Python.h" rather than all the
individual header files.)

- Robert


More information about the cython-devel mailing list