8000 BUG: Move ctypes ImportError catching to appropriate place by davidjn · Pull Request #8898 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: Move ctypes ImportError catching to appropriate place #8898

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

Merged
merged 15 commits into from
May 7, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions numpy/core/_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@

from numpy.compat import basestring
from .multiarray import dtype, array, ndarray
import ctypes
try:
import ctypes
except ImportError:
ctypes = None
from .numerictypes import object_

if (sys.byteorder == 'little'):
Expand Down Expand Up @@ -194,19 +197,33 @@ def _commastring(astr):

return result

class dummy_ctype(object):
def __init__(self, cls):
self._cls = cls
def __mul__(self, other):
return self
def __call__(self, *other):
return self._cls(other)
def __eq__(self, other):
return self._cls == other._cls

def _getintp_ctype():
val = _getintp_ctype.cache
if val is not None:
return val
char = dtype('p').char
if (char == 'i'):
val = ctypes.c_int
elif char == 'l':
val = ctypes.c_long
elif char == 'q':
val = ctypes.c_longlong
if ctypes is None:
import numpy as np
val = dummy_ctype(np.intp)
else:
val = ctypes.c_long
char = dtype('p').char
if (char == 'i'):
val = ctypes.c_int
elif char == 'l':
val = ctypes.c_long
elif char == 'q':
val = ctypes.c_longlong
else:
val = ctypes.c_long
_getintp_ctype.cache = val
return val
_getintp_ctype.cache = None
Expand All @@ -222,9 +239,9 @@ def c_void_p(self, num):

class _ctypes(object):
def __init__(self, array, ptr=None):
try:
if ctypes:
self._ctypes = ctypes
except ImportError:
else:
self._ctypes = _missing_ctypes()
self._arr = array
self._data = ptr
Expand All @@ -250,14 +267,10 @@ def get_data(self):
return self._data

def get_shape(self):
if self._zerod:
return None
return (_getintp_ctype()*self._arr.ndim)(*self._arr.shape)
return self.shape_as(_getintp_ctype())

def get_strides(self):
if self._zerod:
return None
return (_getintp_ctype()*self._arr.ndim)(*self._arr.strides)
return self.strides_as(_getintp_ctype())

def get_as_parameter(self):
return self._ctypes.c_void_p(self._data)
Expand Down
22 changes: 22 additions & 0 deletions numpy/core/tests/test_multiarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -6726,6 +6726,28 @@ def test_null_inside_ustring_array_is_truthy(self):
a[0] = ' \0 \0'
self.assertTrue(a)


class TestCTypes(TestCase):

def test_ctypes_is_available(self):
test_arr = np.array([[1, 2, 3], [4, 5, 6]])

self.assertEqual(ctypes, test_arr.ctypes._ctypes)
assert_equal(tuple(test_arr.ctypes.shape), (2, 3))

def test_ctypes_is_not_available(self):
from numpy.core import _internal
_internal.ctypes = None
try:
test_arr = np.array([[1, 2, 3], [4, 5, 6]])

self.assertIsInstance(
test_arr.ctypes._ctypes, _internal._missing_ctypes)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debatable whether testing implementation details like this (and the one above) makes any sense

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you feel strongly, I'll remove it. I think its a good check to keep. For instance, if someone decides to add an "import ctypes" statement into one of these methods in the future, this assertion will catch that. Otherwise, the test will continue to pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, my concernt is that that's somewhat artificial, and doesn't really catch us doing import ctypes anywhere else - these tests wouldn't catch the bug that caused you to patch this in the first place, for instance.

I don't feel strongly about it, but would appreciate input from someone else on a better way to test this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Will wait for additional input.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm almost tempted to spin off a new python interpreter without ctypes to run the test, to ensure it can't break anything else

You might also be able to get your sys.modules hackery to work if you try to put everything back together again, but also a little risky.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Eric,

I tried again with the sys.modules hackery, and trying to put things back together again with the try/finally block. Unfortunately, other tests continue to fail. I guess this is because tests are run in parallel. With the current setup (setting ctypes = None), we will have similar cross-test side effects, but I guess we are getting lucky in that it isn't causing any errant failures.

By "spin off a new python interpreter", are you suggesting to execute the test with python's subprocess?

Eg (?):
def some_test(self):
self.assertEqual(0, subprocess.call(sys.executable, "-c", test_code)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggesting to execute the test with python's subprocess?

Yes, that is what I mean. I'm not actually sure that's a good idea though, and I don't think you should risk wasting time trying it until someone else weighs in.

assert_equal(tuple(test_arr.ctypes.shape), (2, 3))
finally:
_internal.ctypes = ctypes


def test_orderconverter_with_nonASCII_unicode_ordering():
# gh-7475
a = np.arange(5)
Expand Down
0