[Cython] Cannot cythonize subclasses of setuptools.extension._Extension

[Manuel Nuno Melo]

Hello devs,

I'm developing the setup.py for a scientific package, MDAnalysis
 (see PR #799
). We depend on
distutils and setuptool. Namely, we use setuptools.extension.Extension
class for our extensions.

Some older versions of setuptools (<18.0) do filename cythonization
themselves upon initialization of the Extension object.

Because we want to control name cythonization ourselves I try to directly
use distutils.extension.Extension, which has none of setuptools'
cythonization. However, this doesn't work because setuptools patches
distutils, so that distutils.extension.Extension effectively becomes
setuptools.extension.Extension.

setuptools does provide a copy of the unpatched Extension class, via
setuptools.extension._Extension. Yet, if I attempt to pass instances of
this class to Cython.cythonize it fails because my extension parent no
longer conforms to one of the expected classes:

  File "./setup.py", line 283, in cythonize
new_ext_modules = Cython.cythonize(self.ext_modules)
  File
"/scratch/virtualenv/lib/python2.7/site-packages/Cython-0.24-py2.7-linux-x86_64.egg/Cython/Build/Dependencies.py",
line 796, in cythonize
aliases=aliases)
  File
"/scratch/virtualenv/lib/python2.7/site-packages/Cython-0.24-py2.7-linux-x86_64.egg/Cython/Build/Dependencies.py",
line 686, in create_extension_list
raise TypeError(msg)
TypeError: pattern is not of type str nor subclass of Extension () but of type  and class distutils.extension.Extension

I believe cythonize should be more permissive with the extension parent
classes it accepts, or at least have the setuptools _Extension one in the
whitelist, since it's the only way to access the original distutils class.
What do you think?

In the meantime I'll solve this by subclassing
setuptools.extension.Extension and making sure it doesn't cythonize
anything.

Cheers,
Manuel
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel

Re: [Cython] Cannot cythonize subclasses of setuptools.extension._Extension

[Manuel Nuno Melo]

Our need to control cythonization comes from the fact that we implement
cython as a lazy and optional dependency. Lazy in the sense that we delay
as much as possible cythonization so that setuptools or pip have time to
install cython, if needed. Optional because we distribute both .pyx and
cythonized .c files, and decide on which to use based on user flags.

We, therefore, need an Extension class that only cythonizes if we decide to.

Thanks for the feedback,
Manel

On Apr 14, 2016 8:17 PM, "Matthew Brett"  wrote:
>
> On Thu, Apr 14, 2016 at 6:08 AM, Erik Bray  wrote:
> > On Wed, Apr 13, 2016 at 9:35 PM, Manuel Nuno Melo
> >  wrote:
> >> Hello devs,
> >>
> >> I'm developing the setup.py for a scientific package, MDAnalysis (see
PR
> >> #799). We depend on distutils and setuptool. Namely, we use
> >> setuptools.extension.Extension class for our extensions.
> >>
> >> Some older versions of setuptools (<18.0) do filename cythonization
> >> themselves upon initialization of the Extension object.
> >>
> >> Because we want to control name cythonization ourselves I try to
directly
> >> use distutils.extension.Extension, which has none of setuptools'
> >> cythonization. However, this doesn't work because setuptools patches
> >> distutils, so that distutils.extension.Extension effectively becomes
> >> setuptools.extension.Extension.
> >
> > I'm wondering what it is specifically you need to do in your
> > subclass--might it still be possible to do with a subclass of the
> > setuptools Extension?  Not saying I disagree with the overall idea,
> > but I also wonder if there isn't a better way.
>
> I know this is a terrible and ugly hack, but the projects I work in
> have a 'fake_pyrex' directory, that fools setuptools into thinking
> that 'pyrex' is installed, and therefore prevents it from doing the
> .pyx -> .c filename conversions in the extension:
>
> https://github.com/regreg/regreg/blob/master/setup.py#L33
>
> Cheers,
>
> Matthew
> ___
> 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

Re: [Cython] Cannot cythonize subclasses of setuptools.extension._Extension

[Manuel Nuno Melo]

Hi Erik,
Please post your solution; I'm curious to see it.

Currently, we're also using setup_requires but are moving away from it
because of two main issues:

1- there's no flexibility to hook into it via setuptools.Distribution
subclassing (unless you rewrite the entire __init__);

2- (and more serious) setuptools installs setup_requires dependencies in
the local directory, not as a system-wide install. Even worse, it then puts
a path link to the current directory under the system-wide
easy_install.pth. This means that if you install from the source directory
via 'sudo ./setup.py install', you get a Cython egg downloaded into that
directory, and its path added to sys.path (!!). Needless to say this can
break in many nasty ways...

On Sat, Apr 16, 2016 at 1:10 PM, Erik Bray  wrote:

> On Apr 14, 2016 21:07, "Manuel Nuno Melo" 
> wrote:
> >
> > Our need to control cythonization comes from the fact that we implement
> cython as a lazy and optional dependency. Lazy in the sense that we delay
> as much as possible cythonization so that setuptools or pip have time to
> install cython, if needed. Optional because we distribute both .pyx and
> cythonized .c files, and decide on which to use based on user flags.
> >
> > We, therefore, need an Extension class that only cythonizes if we decide
> to.
> >
> > Thanks for the feedback,
> > Manel
>
> I have a solution for exactly this in astropy_helpers which makes it
> possible, for example, to pull in Cython via setup_requires, but only if
> it's needed.
>
> Remind me on Monday and I can point out how it works.
>
> > On Apr 14, 2016 8:17 PM, "Matthew Brett" 
> wrote:
> > >
> > > On Thu, Apr 14, 2016 at 6:08 AM, Erik Bray 
> wrote:
> > > > On Wed, Apr 13, 2016 at 9:35 PM, Manuel Nuno Melo
> > > >  wrote:
> > > >> Hello devs,
> > > >>
> > > >> I'm developing the setup.py for a scientific package, MDAnalysis
> (see PR
> > > >> #799). We depend on distutils and setuptool. Namely, we use
> > > >> setuptools.extension.Extension class for our extensions.
> > > >>
> > > >> Some older versions of setuptools (<18.0) do filename cythonization
> > > >> themselves upon initialization of the Extension object.
> > > >>
> > > >> Because we want to control name cythonization ourselves I try to
> directly
> > > >> use distutils.extension.Extension, which has none of setuptools'
> > > >> cythonization. However, this doesn't work because setuptools patches
> > > >> distutils, so that distutils.extension.Extension effectively becomes
> > > >> setuptools.extension.Extension.
> > > >
> > > > I'm wondering what it is specifically you need to do in your
> > > > subclass--might it still be possible to do with a subclass of the
> > > > setuptools Extension?  Not saying I disagree with the overall idea,
> > > > but I also wonder if there isn't a better way.
> > >
> > > I know this is a terrible and ugly hack, but the projects I work in
> > > have a 'fake_pyrex' directory, that fools setuptools into thinking
> > > that 'pyrex' is installed, and therefore prevents it from doing the
> > > .pyx -> .c filename conversions in the extension:
> > >
> > > https://github.com/regreg/regreg/blob/master/setup.py#L33
> > >
> > > Cheers,
> > >
> > > Matthew
> > > ___
> > > 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

Re: [Cython] Cannot cythonize subclasses of setuptools.extension._Extension

[Manuel Nuno Melo]

Ah, sorry Erik, you're absolutely right. I mixed my results a bit and must
elaborate:

'setup_requires' on its own will indeed not generate the behavior I
described.

However, if you define the same dependency under 'setup_requires' AND
'install_requires', then you get the mess I mentioned. Essentially, when
you reach 'install_requires' setuptools decides that the egg it has under
.eggs suffices, and just links to it.

This isn't an issue when depending on cython because it's really just a
setup-time dependency. However, in our package we depend on numpy at both
setup- and runtime, and therefore makes sense to have it under both
'requires' flags.

This is why hooking in at the setup_requires step could be a potential
workaround, if we could get it to do a full dependency install instead of
local egg. I'm currently finishing up this approach.

On Mon, Apr 18, 2016 at 11:16 AM, Erik Bray  wrote:

> On Sat, Apr 16, 2016 at 1:29 PM, Manuel Nuno Melo
>  wrote:
> > Hi Erik,
> > Please post your solution; I'm curious to see it.
>
> Will do in a bit. I need to see if I can distill it somewhat from its
> original context so that it can be better understood.
>
> > Currently, we're also using setup_requires but are moving away from it
> > because of two main issues:
> >
> > 1- there's no flexibility to hook into it via setuptools.Distribution
> > subclassing (unless you rewrite the entire __init__);
>
> I don't really think that's a big issue but maybe you've encountered
> some reason that it is.
> The worse thing about it is the general chicken-egg problem that you
> need to call setup() to get your setup_requires, but you may need some
> packages pulled in from setup_requires in order to do anything useful
> with setup().
>
> d2to1 [1] solves this problem quite well, but unfortunately
> development on it is stalled for the time being.  Another alternative
> that can work quite well is some version of Daniel Holth's
> "setup-requires" hack [2].
>
> > 2- (and more serious) setuptools installs setup_requires dependencies in
> the
> > local directory, not as a system-wide install.
>
> That's a feature, not a bug.  Build requirements for a package might
> be in conflict with the versions of those same requirements you have
> already installed.  It also prevents things from being installed that
> are *not* required at runtime.  It makes it easier for projects to pin
> a little more strictly to their known-working build requirements,
> without the constraints imposed by supporting a wider range of runtime
> requirements.
>
> > Even worse, it then puts a
> > path link to the current directory under the system-wide
> easy_install.pth.
> > This means that if you install from the source directory via 'sudo
> > ./setup.py install', you get a Cython egg downloaded into that directory,
> > and its path added to sys.path (!!). Needless to say this can break in
> many
> > nasty ways...
>
> Huh??  I'm not sure we're talking about the same thing now.  The local
> .eggs cache directory created for setup_requires downloads most
> certainly does not get added to easy_install.pth.  Are you sure you're
> not thinking of `setup.py develop`?
>
> [1] https://pypi.python.org/pypi/d2to1
> [2] https://bitbucket.org/dholth/setup-requires/src
>
> > On Sat, Apr 16, 2016 at 1:10 PM, Erik Bray 
> wrote:
> >>
> >> On Apr 14, 2016 21:07, "Manuel Nuno Melo" 
> >> wrote:
> >> >
> >> > Our need to control cythonization comes from the fact that we
> implement
> >> > cython as a lazy and optional dependency. Lazy in the sense that we
> delay as
> >> > much as possible cythonization so that setuptools or pip have time to
> >> > install cython, if needed. Optional because we distribute both .pyx
> and
> >> > cythonized .c files, and decide on which to use based on user flags.
> >> >
> >> > We, therefore, need an Extension class that only cythonizes if we
> decide
> >> > to.
> >> >
> >> > Thanks for the feedback,
> >> > Manel
> >>
> >> I have a solution for exactly this in astropy_helpers which makes it
> >> possible, for example, to pull in Cython via setup_requires, but only if
> >> it's needed.
> >>
> >> Remind me on Monday and I can point out how it works.
> >>
> >> > On Apr 14, 2016 8:17 PM, "Matthew Brett" 
> >> > wrote:
> >> > >
> >> > > On Thu, Apr 

Re: [Cython] Cannot cythonize subclasses of setuptools.extension._Extension

[Manuel Nuno Melo]

This strayed a bit from the original topic, but yes, I should bring this up
with setuptools project. Thanks for the feedback.
(My feeling was that this was a somewhat stalled problem, reported in two
issues and persisting after a couple of years:
#209
<https://bitbucket.org/pypa/setuptools/issues/209/setup_requires-and-install_requires-dont>
and #391
<https://bitbucket.org/pypa/setuptools/issues/391/dependencies-listed-in-both-setup_requires>
)

As you said, even if it is a hackish workaround here, Cython should still
allow setuptools.extension._Extension.

On Tue, Apr 19, 2016 at 11:48 AM, Erik Bray  wrote:

> On Mon, Apr 18, 2016 at 6:56 PM, Manuel Nuno Melo
>  wrote:
> > Ah, sorry Erik, you're absolutely right. I mixed my results a bit and
> must
> > elaborate:
> >
> > 'setup_requires' on its own will indeed not generate the behavior I
> > described.
> >
> > However, if you define the same dependency under 'setup_requires' AND
> > 'install_requires', then you get the mess I mentioned. Essentially, when
> you
> > reach 'install_requires' setuptools decides that the egg it has under
> .eggs
> > suffices, and just links to it.
> >
> > This isn't an issue when depending on cython because it's really just a
> > setup-time dependency. However, in our package we depend on numpy at both
> > setup- and runtime, and therefore makes sense to have it under both
> > 'requires' flags.
> >
> > This is why hooking in at the setup_requires step could be a potential
> > workaround, if we could get it to do a full dependency install instead of
> > local egg. I'm currently finishing up this approach.
>
> Interesting.  This sounds to me like a regression, as I don't think it
> used to do that.  This should probably be brought up on distutils-sig
> or in the setuptools bug tracker, rather than spending a lot of time
> hacking together a workaround or requiring changes specifically in
> Cython (though I don't disagree that distutils.Extension should be
> allowed in the first place).
>
> IIRC it used to be that if the same package was specified in both
> setup_requires and install_requires it would be built/installed twice.
> This was recognized as inefficient, and that the copy installed in the
> local .eggs directory satisfies an install_requires requirement it
> should be copied from .eggs to the system site-packages (or whatever
> the install target is).  I remember there at least being *discussion*
> about that, but not sure what changes were made.
>
> It sounds like in your case it's just adding Cython from your .eggs
> into easy-install.pth which I agree is wrong.  If that's the case it
> should be fixed in setuptools over all else.  I'll see if I can
> reproduce it.  Jason Coombs who mantains setuptools is usually quick
> to release bug fix versions if he gets a patch.
>
> Erik
>
> > On Mon, Apr 18, 2016 at 11:16 AM, Erik Bray 
> wrote:
> >>
> >> On Sat, Apr 16, 2016 at 1:29 PM, Manuel Nuno Melo
> >>  wrote:
> >> > Hi Erik,
> >> > Please post your solution; I'm curious to see it.
> >>
> >> Will do in a bit. I need to see if I can distill it somewhat from its
> >> original context so that it can be better understood.
> >>
> >> > Currently, we're also using setup_requires but are moving away from it
> >> > because of two main issues:
> >> >
> >> > 1- there's no flexibility to hook into it via setuptools.Distribution
> >> > subclassing (unless you rewrite the entire __init__);
> >>
> >> I don't really think that's a big issue but maybe you've encountered
> >> some reason that it is.
> >> The worse thing about it is the general chicken-egg problem that you
> >> need to call setup() to get your setup_requires, but you may need some
> >> packages pulled in from setup_requires in order to do anything useful
> >> with setup().
> >>
> >> d2to1 [1] solves this problem quite well, but unfortunately
> >> development on it is stalled for the time being.  Another alternative
> >> that can work quite well is some version of Daniel Holth's
> >> "setup-requires" hack [2].
> >>
> >> > 2- (and more serious) setuptools installs setup_requires dependencies
> in
> >> > the
> >> > local directory, not as a system-wide install.
> >>
> >> That's a feature, not a bug.  Build requirements for a package might
> >> be in conflict with the versions of those same