Merge lp:~cwilson/spud/pythonbug into lp:spud

Proposed by Cian Wilson
Status: Merged
Approved by: Patrick Farrell
Approved revision: 405
Merged at revision: 407
Proposed branch: lp:~cwilson/spud/pythonbug
Merge into: lp:spud
Diff against target: 94 lines (+45/-6)
2 files modified
python/libspud.py.in (+35/-5)
python/test_ctypes.py (+10/-1)
To merge this branch: bzr merge lp:~cwilson/spud/pythonbug
Reviewer Review Type Date Requested Status
Patrick Farrell Approve
Review via email: mp+66989@code.launchpad.net

Description of the change

This merge fixes a mangling/out of range issue (for non-square tensors) with get_option in the python libspud interface (bug lp:806331).

Additionally it adds functionality to allow set_option to work for rank>0 objects (bug lp:806332).

Test added to test_ctypes.py.

To post a comment you must log in.
lp:~cwilson/spud/pythonbug updated
405. By Cian Wilson

Oops, removing duplicated derived exception class.

Revision history for this message
Patrick Farrell (pefarrell) wrote :

Looks good. It would be nice if we did numpy arrays too, but it's not essential. If you're clever about your try/exceptions, it doesn't have to have a hard dependency on numpy.

Revision history for this message
Cian Wilson (cwilson) wrote :

True but I'll leave that until it bugs me enough to fix (probably won't take that long).

Thanks Patrick.

Revision history for this message
Florian Rathgeber (florian-rathgeber) wrote :

Patrick, do you still have plans to use numpy arrays for this? Otherwise this should be approved (since it has been merged already) and the 3 related issues closed.

Revision history for this message
Patrick Farrell (pefarrell) wrote :

I approve. I don't need the numpy arrays, so I'm not pushed about them.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'python/libspud.py.in'
--- python/libspud.py.in 2011-07-05 13:54:45 +0000
+++ python/libspud.py.in 2011-07-06 05:48:34 +0000
@@ -173,7 +173,8 @@
173 val_type = ctype_map[type]173 val_type = ctype_map[type]
174 rank = get_option_rank(s)174 rank = get_option_rank(s)
175 shape = get_option_shape(s)175 shape = get_option_shape(s)
176 for i in range(rank): val_type = val_type*shape[i]176 # reverse the numbering so we get a rowxcolumn list of lists
177 for i in range(rank-1, -1, -1): val_type = val_type*shape[i]
177 val = val_type()178 val = val_type()
178179
179 out = cget_option(s, byref(c_int(len(s))), byref(val))180 out = cget_option(s, byref(c_int(len(s))), byref(val))
@@ -205,19 +206,48 @@
205cset_option.restype = c_int206cset_option.restype = c_int
206207
207def set_option(s, val):208def set_option(s, val):
209 rank = 0
210 shape_type = c_int * 2
211 shape = shape_type(1, -1)
208 py_type = type(val)212 py_type = type(val)
213 if py_type not in typepy_map:
214 if py_type == list:
215 subval = val
216 for i in range(2):
217 if type(subval) == list:
218 shape[i] = len(subval)
219 rank += 1
220 subval = subval[0]
221 else:
222 break
223 py_type = type(subval)
224 # elif ...
225 # could put numpy array handling in here
226 # (they'd be easier but didn't want to
227 # add a numpy dependency)
228 else:
229 raise spud_exceptions[SPUD_TYPE_ERROR]("Unknown value type. Only know about floats, ints, strings and lists thereof.")
209 spud_code = typepy_map[py_type]230 spud_code = typepy_map[py_type]
210 c_type = ctype_map[py_type]231 c_type = ctype_map[py_type]
211 c_val = c_type(val)232 # reverse the order of the shape entries as that way we end up with a rows x columns c_type
233 for i in range(rank-1, -1, -1): c_type = c_type*shape[i]
234 if rank == 2:
235 c_val = c_type()
236 for i in range(shape[0]):
237 for j in range(shape[1]):
238 c_val[i][j] = val[i][j]
239 elif rank == 1:
240 c_val = c_type()
241 for i in range(shape[0]):
242 c_val[i] = val[i]
243 else:
244 c_val = c_type(val)
212245
213 shape_type = c_int * 2
214 if py_type is str:246 if py_type is str:
215 shape = shape_type(len(val), -1)247 shape = shape_type(len(val), -1)
216 rank = 1248 rank = 1
217 out = cset_option(s, byref(c_int(len(s))), (c_val), byref(c_int(spud_code)), byref(c_int(rank)), shape)249 out = cset_option(s, byref(c_int(len(s))), (c_val), byref(c_int(spud_code)), byref(c_int(rank)), shape)
218 else:250 else:
219 shape = shape_type(1, -1)
220 rank = 0
221 out = cset_option(s, byref(c_int(len(s))), byref(c_val), byref(c_int(spud_code)), byref(c_int(rank)), shape)251 out = cset_option(s, byref(c_int(len(s))), byref(c_val), byref(c_int(spud_code)), byref(c_int(rank)), shape)
222252
223 if out != SPUD_NO_ERROR:253 if out != SPUD_NO_ERROR:
224254
=== modified file 'python/test_ctypes.py'
--- python/test_ctypes.py 2011-07-05 13:54:45 +0000
+++ python/test_ctypes.py 2011-07-06 05:48:34 +0000
@@ -30,13 +30,22 @@
30print libspud.get_option_rank(list_path)30print libspud.get_option_rank(list_path)
31print libspud.get_option(list_path)31print libspud.get_option(list_path)
32assert(libspud.get_option(list_path)==[7,8,9,10])32assert(libspud.get_option(list_path)==[7,8,9,10])
33libspud.set_option(list_path, [11,12,13,14])
34print libspud.get_option_shape(list_path)
35print libspud.get_option_rank(list_path)
36print libspud.get_option(list_path)
37assert(libspud.get_option(list_path)==[11,12,13,14])
3338
34tensor_path = '/material_phase::Material1/tensor_field::DummyTensor/prescribed/value::WholeMesh/anisotropic_asymmetric/constant'39tensor_path = '/material_phase::Material1/tensor_field::DummyTensor/prescribed/value::WholeMesh/anisotropic_asymmetric/constant'
35print libspud.get_option_shape(tensor_path)40print libspud.get_option_shape(tensor_path)
36print libspud.get_option_rank(tensor_path)41print libspud.get_option_rank(tensor_path)
37print libspud.get_option(tensor_path)42print libspud.get_option(tensor_path)
38assert(libspud.get_option(tensor_path)==[[1.0,2.0],[3.0,4.0]])43assert(libspud.get_option(tensor_path)==[[1.0,2.0],[3.0,4.0]])
3944libspud.set_option(tensor_path, [[5.0,6.0],[7.0,8.0]])
45print libspud.get_option_shape(tensor_path)
46print libspud.get_option_rank(tensor_path)
47print libspud.get_option(tensor_path)
48assert(libspud.get_option(tensor_path)==[[5.0,6.0],[7.0,8.0]])
4049
41try:50try:
42 libspud.add_option('/foo')51 libspud.add_option('/foo')

Subscribers

People subscribed via source and target branches