Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#298 Make wrapping work with numpy 1.7 #373

Merged
merged 4 commits into from
Mar 21, 2013
Merged

#298 Make wrapping work with numpy 1.7 #373

merged 4 commits into from
Mar 21, 2013

Conversation

rmjarvis
Copy link
Member

I think I have everything working with numpy 1.7 now.

I have to say, now that I've gone through this that their API is complete crap. And it has only gotten worse with this new change. You basically have to reinterpret_cast everything back and forth between PyObject* and PyArrayObject*. And I'm sorry, but anything that requires that much use of reinterpret_cast as a matter of course just to do the really normal things you would want to use it for is extremely poor design.

Most of what I had to do was add even more explicit casts to make the arguments to various functions have the right type. They used to just work when everything was a PyObject* (although Jim already had a fair amount of other casting going on with the old API), but now they give errors if you pass a PyObject* to most of the functions that want a PyArrayObject*, even though their API documentation says "The input argument, arr, can be any PyObject * that is directly interpretable as a PyArrayObject * (any instance of the PyArray_Type and its sub-types)." But apparently that's just bullshit.

Anyway, sorry for the rant. This is a pretty easy pull request. I've already tested it on systems that have numpy versions 1.2, 1.5, 1.6 and 1.7. So I think everything is copasetic. But give it a try to make sure it works on your system.

@rmandelb
Copy link
Member

Wow Mike, that's awfully waffly - tell us how you really feel! ;)
Works on both my systems, looks good. Thanks for taking care of this one!

@reikonakajima
Copy link
Member

Looks good on my 4 systems too.

@dkirkby
Copy link
Member

dkirkby commented Mar 20, 2013

For what its worth, I just tried building GalSim against NumPy 1.8 and it failed in the scons check even when I use this branch (but reverting to 1.7 worked for me):

Checking if we can build against NumPy... 
Unable to compile a file with numpy using the include path:
/Library/Python/2.7/site-packages/numpy/core/include.

@rmandelb
Copy link
Member

Well it is weird that it would fail at compilation. Is 1.8 a developmental version? It's not in the list of official releases that I found. If it is just in development, then I think we don't have to worry about trying to fix that error (since in any case they could change it further before it gets officially released). However, it is good to have a heads up that we might have an issue with this in the future, so thanks for pointing it out.

On Mar 20, 2013, at 9:36 AM, dkirkby notifications@github.com wrote:

For what its worth, I just tried building GalSim against NumPy 1.8 and it failed in the scons check even when I use this branch (but reverting to 1.7 worked for me):

Checking if we can build against NumPy...
Unable to compile a file with numpy using the include path:
/Library/Python/2.7/site-packages/numpy/core/include.

Reply to this email directly or view it on GitHub.


Rachel Mandelbaum
rmandelb@andrew.cmu.edu
http://www.andrew.cmu.edu/~rmandelb/

@dkirkby
Copy link
Member

dkirkby commented Mar 20, 2013

Yes, I was using the numpy dev snapshot from github. Given that this PR adds a line to SConstruct that refers specifically to 1.7, I wouldn't be surprised if the final 1.8 fails in the same way, but that's a problem for another day.

@rmjarvis
Copy link
Member Author

I just tried building GalSim against NumPy 1.8 and it failed

I think numpy 1.8 is currently broken. Here is the end of the config.log file when I just tried it:

.sconf_temp/conftest_15.cpp <-
  |
  |#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
  |#include "Python.h"
  |#include "numpy/arrayobject.h"
  | 
  |static void doImport() {
  |    import_array();
  |}
  |
  |static PyObject* run(PyObject* self, PyObject* args)
  |{ 
  |    doImport();
  |    int result = 1;
  |    if (!PyErr_Occurred()) {
  |        npy_intp dims = 2;
  |        PyObject* a = PyArray_SimpleNew(1, &dims, NPY_INT);
  |        if (a) {
  |            Py_DECREF(a);
  |            result = 23; 
  |        }
  |    }
  |    return Py_BuildValue("i", result); 
  |}
  |
  |static PyMethodDef Methods[] = {
  |    {"run",  run, METH_VARARGS, "return 23"},
  |    {NULL, NULL, 0, NULL}
  |};
  |
  |PyMODINIT_FUNC initcheck_numpy(void)
  |{ Py_InitModule("check_numpy", Methods); }
  |
g++ -o .sconf_temp/conftest_15.o -c -O2 -fno-strict-aliasing -Wall -Werror -Iinclude -I/home/mjarvis/g++-install/include -I/home/mjarvis/include -I/usr/include/python2.6 -I/data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include .sconf_temp/conftest_15.cpp
In file included from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/ndarraytypes.h:1726,
                 from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/ndarrayobject.h:17,
                 from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/arrayobject.h:4,
                 from .sconf_temp/conftest_15.cpp:4:
/data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/npy_deprecated_api.h:11:2: error: #warning "Using deprecated NumPy API, disable it by #defining NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION"
/data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/npy_deprecated_api.h:23:2: error: #error Should never include npy_deprecated_api directly.
In file included from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/npy_deprecated_api.h:126,
                 from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/ndarraytypes.h:1726,
                 from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/ndarrayobject.h:17,
                 from /data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/arrayobject.h:4,
                 from .sconf_temp/conftest_15.cpp:4:
/data2/home/mjarvis/lib64/python2.6/site-packages/numpy/core/include/numpy/old_defines.h:6:2: error: #error The header "old_defines.h" is deprecated as of NumPy 1.7.

So it gives errors about including old_defines.h and npy_deprecated_api.h, both of which come from their file, numpy/arrayobject.h. So I have no idea what they expect us to do for that. For now, I think we need to just say we don't support numpy 1.8. Hopefully they'll fix this before they release it.

@rmjarvis
Copy link
Member Author

FYI, I updated the FAQ entry about this problem.

rmjarvis added a commit that referenced this pull request Mar 21, 2013
#298 Make wrapping work with numpy 1.7
@rmjarvis rmjarvis merged commit 94b605b into master Mar 21, 2013
@rmandelb rmandelb deleted the #298 branch July 4, 2014 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants