Remove PyCXX dependency for core extension modules #3646
Conversation
|
Wow! Thank you for executing this massive task! |
|
Got it to compile (not tested) on Windows with msvc9 and msvc10 with the following changes: diff --git a/src/ft2font.cpp b/src/ft2font.cpp
index bc242fd..6e7e0a2 100644
--- a/src/ft2font.cpp
+++ b/src/ft2font.cpp
@@ -622,7 +622,7 @@ int FT2Font::get_kerning(int left, int right, int mode)
void FT2Font::set_text(
size_t N, uint32_t *codepoints, double angle, FT_UInt32 flags, std::vector<double> &xys)
{
- angle = angle / 360.0 * 2 * M_PI;
+ angle = angle / 360.0 * 2 * 3.14159265358979323846;
// this computes width and height in subpixels so we have to divide by 64
matrix.xx = (FT_Fixed)(cos(angle) * 0x10000L);
diff --git a/src/mplutils.h b/src/mplutils.h
index a0160e0..ab6e7f3 100644
--- a/src/mplutils.h
+++ b/src/mplutils.h
@@ -63,4 +63,10 @@ inline T min(const T &a, const T &b)
#endif
+#ifdef _MSC_VER && _MSC_VER <= 1600
+typedef unsigned __int8 uint8_t;
+#else
+#include <stdint.h>
+#endif
+
#endif
diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h
index dc5b25f..cf91767 100644
--- a/src/numpy_cpp.h
+++ b/src/numpy_cpp.h
@@ -116,6 +116,7 @@ struct type_num_of<npy_double>
value = NPY_DOUBLE
};
};
+#if NPY_LONGDOUBLE != NPY_DOUBLE
template <>
struct type_num_of<npy_longdouble>
{
@@ -123,6 +124,7 @@ struct type_num_of<npy_longdouble>
value = NPY_LONGDOUBLE
};
};
+#endif
template <>
struct type_num_of<npy_cfloat>
{
@@ -158,6 +160,7 @@ struct type_num_of<npy_clongdouble>
value = NPY_CLONGDOUBLE
};
};
+#if NPY_CLONGDOUBLE != NPY_CDOUBLE
template <>
struct type_num_of<std::complex<npy_longdouble> >
{
@@ -165,6 +168,7 @@ struct type_num_of<std::complex<npy_longdouble> >
value = NPY_CLONGDOUBLE
};
};
+#endif
template <>
struct type_num_of<PyObject *>
{
|
|
@mdboom: It should not be too difficult to switch |
|
Looks great! I gave it a quick test on my machine. The code compiles and installs but unfortunately it segfaults during the test suite at more or less random times. I haven't had time to dig more into this yet. This is on OS X mavericks with Xcode 6.0 (clang 3.5 svn) |
|
@ianthomas23: Ah, I was really hoping to get out of some work... No such luck. I'll try to tackle it. @cgohlke: Thanks for the diff. I've incorporated the changes, with some minor changes to remain compatible with clang. @jenshnielsen: I was testing with clang 3.4 -- I just upgraded and I'll let you know how I fare with clang 3.5. |
64e5fde to
139a0e4
Compare
|
@jenshnielsen: For what it's worth, the tests are passing for me on Mac Mavericks, XCode 6.0.1, using source-compiled python and the system Python. It's possible that this is just the result of some changes made earlier today to fix some segfaults on Travis. Haven't tried other pythons... If you can get a backtrace out of gdb for the segfault, that might point at something. |
|
Yes I will try to have a look. For the record I was testing with homebrew python build with homebrew tk needed for pymol. This seems to have some issues with the tk backend so it is possible that the issue is related to homebrew python. |
6cbe37c to
9f76dc7
Compare
|
This is rebased and has been updated for #3497. |
| @@ -3,174 +3,23 @@ | |||
| /* A rewrite of _backend_agg using PyCXX to handle ref counting, etc.. | |||
|
I can't reproduce the crash on Mac any more. I am seeing the same two tests fail as travis. and |
|
I think I've got |
|
For the reference all the tests are passing for me locally too. |
|
Ok! I'd say this is ready to merge from my end. Better to get this kind of thing merged early for lots of testing and to avoid merge conflicts... |
|
👍 Perhaps someone should test it a bit more with some interactive applications? |
MNT : Remove PyCXX dependency for core extension modules
|
I suspect we are going to spend the next few months tracking down strange seg-faults
|
|
Interesting tidbit -- the binary build is 12MB smaller now. |
I realize that this PR is going to be difficult to review because of its size, but unfortunately, there was no good way to do this in pieces, given how interdependent may of our C++ extensions are. Rather than reading it all through, it would be helpful to just download, compile and run it in as many contexts as possible. Note that it's virtually net even in terms of lines of code ;)
In short, this removes the dependency on PyCXX, and uses the direct Python/C API instead (except for
_tri). It is tested and working on Fedora 20, Ubuntu 14.4, Mac OS-X Mavericks. I need some help with Windows testing, particularly since this pushes the limits of the compiler in some places (@cgohlke?).I skipped the
_trimodule is in part because it's not part of the tangled mess of most of the other extensions, but also because (I think) it's deprecated in favor ofqhull(which already doesn't use PyCXX). But, I must admit, I haven't been following the world of triangulation very closely, so maybe we still need to keep_triaround for the foreseeable future. Cc: @ianthomas23Most of these modules are declared private, so I took the liberty to change APIs when keeping them as-is would have caused a great deal of pain, but in general, this isn't a wholesale "rewrite" of anything, and really is meant to be mostly a refactor. The public Python API is unchanged, unless by accident.
An important side benefit of this work is to decoupling the extensions. For instance, in master, the
_imagemodule passesImageobjects to the_backend_aggmodule for drawing (this also happens for fonts and text paths). Now, instead, Numpy arrays are used. This not only decouples the extensions from one another, it means that the data passed back and forth is a Numpy array and thus can be manipulated from Python much easier (Numpy arrays have a gazillion methods, which our dumb image buffers did not) and with fewer copies in some cases. So a lot of the methods here likergba_to_argbetc. should becomenp.rolletc. There's still more work to be done there, and I think that in the long term these sorts of classes should completely disappear. As it stands, there are already some real benefits, such as saving "upside down" images to a PDF/SVG no longer requires a copy. Another nice side effect is that the "flipud_out" method, which statefully flips the stride orientation of anImageis now gone, since it's much easier/better to do that non-statefully using a Numpy array view. I plan to refactor this further when I tackle image resampling.The following methods are gone, since they are just debugging tools, and now that we use Numpy arrays for buffers, it's much easier to poke and prod at the data as Numpy arrays:
Thanks to Mark Weibe for his
numpy_cppproject on which some of this is based. He agrees that given its small size and matplotlib's somewhat unique needs, it was best to just fork it and include it in matplotlib, so that's what I've done. At this point, it bears only a subtle resemblance to the original.