Phillip J. Eby wrote:
At 08:00 PM 9/5/2007 +0200, Stefan Behnel wrote:
Phillip J. Eby wrote:
At 08:30 AM 9/5/2007 +0200, Stefan Behnel wrote:
Perhaps you'd care to produce a patch to implement that "cleaner step"? It's not at all obvious to me how to do that without introducing instability that would be unsuitable for an 0.6cN release.
One way of implementing the above change would be to move the replacement code into build_ext rather than Extension. Something like the (untested) build_ext-patcher.py I attached. Note the type check that tests for build_ext being subclassed.
You're illustrating my point. It's easy to hand-wave about how it should be done, but not so easy to actually *do*. Did you look at where all the .sources attribute gets used? How the build_ext command can get called by other commands?
Sort of. Do you doubt it should work that way?
I know that other commands use the .sources of build_ext indirectly, at a time when run() has not yet been called. That means that your approach may change those other commands' behavior in non-obvious ways.
I bet there's a method that you can safely override to do what I meant. Might take me a few minutes longer to find than you, but I actually doubt I currently want to invest that. You shouldn't forget that it's your code that's broken.
You're also ignoring my larger point: the current mechanism allows you to write setup scripts that *don't* need to subclass build_ext. A setuptools-based setup script just refers to '.pyx' files, and everything else happens automatically.
The script I posted doesn't change that. Read it.
Actually, now that I've read it in detail, I see that it's also broken in other ways, such as confusing modules and classes.
I guess you meant the sys.modules bit. That should be easy to fix. It's untested, as I said. It was just meant to make clear what I was proposing.
But it's also broken in the way I said: it *requires* subclassing of build_ext in setup.py, which the current mechanism does not.
No, it just prevents .pyx file mangling *if* you have subclassed build_ext. "If" meaning: "in the case that", so it's a thing that may be, but doesn't have to be. Not very close to the definition of "requires". *If* build_ext was subclassed, doing nothing is a safe bet, as a subclass can do whatever it likes and should not suffer from setuptools patching stuff it might not even know about. If you do not subclass, it behaves as before (probably minus some problems, as you suggested above).
You're actually forgetting the point you stressed most. If we prevent users from installing Cython and Pyrex at the same time, they will have to start modifying setup.py scripts. So it's pushing the burden on them.
Actually, I think you need to decide whether you are really providing an extended Pyrex compiler. If so, then use .pyx as the extension and include a Pyrex-compatible build_ext module, and then there is no need to have both installed on the same system, and everything works splendidly for everyone.
If on the other hand you are *not* providing a Pyrex replacement and must co-exist, then use a different file extension (.cy, perhaps?) and you get exactly the behavior you wanted -- i.e., setuptools doesn't touch your files, and explicit action is required to change the file extensions.
And there is still a third solution: provide your own Extension class for users to import, since they will have to import from Cython in order to use your extended build_ext class anyway, nothing stops them from importing the Extension type too. (Just have your Extension.__init__ save the sources and restore them after calling the superclass; it'll then work whether the patched version is present or not.)
Patch wars, eh? :) You're still forgetting that that pushes the burden on each writer of a setup.py script. They have to take caution to support Pyrex *and* Cython (*if* they want to, which is currently up to them), and they have to care about the case where none of them is importable to get a build_ext replacement, which is required by both Pyrex and Cython. I still can't see the case where your extension.py patching becomes truly beneficial to anyone. It would be if people really had to do nothing, but that's not the case. But maybe we should just start shipping Cython with a fake Pyrex that people can import and still get Cython instead. Although that doesn't really fix the bug in setuptools. But then, that's your code. Stefan