numpy.place - strange behaviour with array of strings

I am using:

Python 2.7.10 (default, Oct 14 2015, 16:09:02) [GCC 5.2.1 20151010] on linux2
numpy 1.10.1
Linux wojtek-dell 3.13.0-66-generic #108-Ubuntu SMP Wed Oct 7 15:20:27 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

Given this snippet:

import numpy as np
arr = np.array(['12', '34'])
np.place(arr, np.array([False, True]), '9')
print arr

I would imagine that arr will equal ['12' '9'], but instead it is ['12' '94'].
Similar code with numpy.putmaskgives expected ['12' '9'].
Is this a designed behaviour of numpy.place?

From the documentation, it sounds like they are very similar. Underneath the hood, the implementations are slightly different. np.putmask calls memmove with the size_t input equal to the size of an element in arr (method can be found here), whereas np.place calls memcpy with the size_t input equal to the size of an element in values (method can be found here).

That difference (I think) would explain why you were seeing what you saw. By equating the size_t input to the size of an element in arr, you ensure that the element gets completely overwritten in memory, whereas equating the size_t input to the size of an element in values means that only part of an element in arr could be overwritten if the size of the values elements are smaller than those of arr. As you can see from your method call to np.place, the '9' replaced the first element of '34' to get '94'.

If what I'm describing was the intention, then the documentation does not make that clear and should be clarified, though I must say it would get kind of nasty having to describe the C internals. Also, I think that the documentation should clarify how np.place differs from np.putmask given how similar they are.

Sounds like a bug. I would generally suggest to use np.copyto or (in this case) boolean fancy indexing to achieve the same and avoid np.place or np.putmask. I realize that in some cases those functions are not quite 1:1 replaces by these.

EDIT: Of course that does not mean we should not fix it.

This just looks like some broken code to me, a living fossil from the ancient days when it hadn't yet occurred to anyone that one might want to store something besides numbers and objects inside arrays.

putmask already points to copyto (but with some wonky NA details), place should also point to advanced indexing, some easier doc fix for someone who wants to do a good deed for the day.

@seberg

@seberg : IIUC the behavior of np.place and np.copyto are slightly different.

Out of curiosity though, what exactly is the difference supposed to be between np.place and np.putmask? They sound exactly the same from the documentation, especially if the string behavior is in fact a bug.

@seberg

@seberg : I'm not sure I quite follow your comments about documentation. I'm actually planning to put up a PR related to these functions, so your suggestions, once clarified, should certainly be useful.

putmask is like copyto, "indexed"/changed array and "values put in array" are iterated together and wherever it is True (in the mask array, might be reversed for the two functions) the value is copied into the "indexed" array from the "values" array. In other words, "indexed"/changed array and "values" array have the same shape, the shape of the "mask" array is identical, the values inside do not matter. The mention of preserving NA values for copyto is from stuff that had to be ripped out again (but left some residues in places like here).

place is like advanced boolean indexing, which means that the "values" array should have the same length as np.flatnonzero(mask). So the values inside mask matter for the shape of the values array.

I am not quite sure without checking. At least place might be a flattened version, which would mean that the equivalency in some corner cases is more with arr.flat[mask] = values instead of arr[mask] = values (though I am not a big fan of the flat iterator especially in those corner cases....)

EDIT: Had some wrong stuff in here...

@seberg

@seberg Okay, now I see the difference between place and putmask. I should change that in my PR as well. Thanks!

Regarding place: does values have to have the same length as np.flatnonzero(mask)? I tried calling place where that wasn't true, and it handled the call just fine without any errors / warnings.

No, it is like the flat iterator in that regard (I expect). That would mean that the shapes can all be complete nonsense it just iterates them all in C-order (possibly repetitively) until its done.

@seberg

@seberg: should we just rip out the implementations of these things and replace them with the equivalent calls to the functions/indexing that work?

@njsmith

@njsmith I would like it, but you first have to make sure they actually are identical to the flat iterator. np.put is not quite, since it has the kind of "mode" parameter as np.take, but place likely can just point there.
I would still be in favor of pointing people away from them in the doc, the way the flat iterator does things sloppily is a bad idea unless needed IMO.

@seberg

@seberg : Could we instead just enforce shape parameters? Perhaps we start with a RuntimeWarning if the args that don't match in shape and then turn it into an error later?

@njsmith

I'm not so much worried about the flat iterator stuff as the fact that this
code has all these direct calls to memcpy etc. rather than going through
the dtype's PyArray_ArrFuncs structure which is just completely wrong.

On Tue, Jan 12, 2016 at 1:38 AM, seberg [email protected] wrote:

@njsmith https://github.com/njsmith I would like it, but you first have
to make sure they actually are identical to the flat iterator. np.put is
not quite, since it has the kind of "mode" parameter as np.take, but place
likely can just point there.
I would still be in favor of pointing people away from them in the doc,
the way the flat iterator does things sloppily is a bad idea unless
needed IMO.

—
Reply to this email directly or view it on GitHub
#6974 (comment).

Nathaniel J. Smith -- http://vorpus.org

Yeah, doing away with the weird shapes would be a second step. And I think it is likely better to just document the preferred version. For some normal indexing we are in fact doing the deprecation right now.

I think the preferred version is one of the two:

arr[mask] = values
np.copyto(arr, value, where=mask)

Which one depends on what you want to do. Though I admit that subclasses can override place. This to me seems preferable over the actually probably compatible version/replacement of arr.flat[mask] = values in the case of place (there is no compatible replacement for putmask probably.

@njsmith

@njsmith : Just to make sure I'm understanding this properly, when I call np.putmask, are we actually calling that PyArray_FastPutmaskFunc function in the dtype's PyArray_ArrFuncs? I'm just not sure how everything gets linked up here when I do setup.py on numpy because I know eventually it gets stored in a multiarray lib that I cannot read.

@seberg

@seberg : IIUC I think you're referring to np.put.

@gfyoung

@gfyoung: np.putmask is implemented by array_putmask in multiarraymodule.c (how I figured this out: C functions exposed to python are always listed in a structure that contains a C string naming the Python function, so I did git grep '"putmask"' -- notice that this is searching for the string "putmask" with the double-quotes included as part of the search). And array_putmask is a tiny wrapper around the global function PyArray_PutMask, and you can trace it from there.

Another stopgap measure if we don't have an easy way to fix these functions, and don't want to spend the time to rewrite them properly, would be to add a check at the entry that errors out if the array has any dtype aside from the built-in fixed-width ones like int64, so at least users are protected from the code's brokenness.

Is this a bug as well?

>>> import numpy as np
>>> mask = [True, False]
>>> replace = 'FOO'
>>>
>>> m = np.array(['A', 'B'])
>>> np.place(m, mask, replace)
>>> m
array(['F', 'O'],
      dtype='|S1')
>>>
>>> m = np.array(['A', 'B'])
>>> np.copyto(m, replace, where=mask)
>>> m
array(['F', 'O'],
      dtype='|S1')
>>>
>>> np.put(m, [0], replace)
>>> m
array(['F', 'O'],
      dtype='|S1')

It seems like strange things happen when the element size of the copied elements in values / src differ from those in array / dest. In this case, we get this unexpected truncation, which also occurs when you use integers of different sizes (e.g. replace np.int8 with np.int64)

I guess place should do whatever the regular casting rules would do.
On Jan 12, 2016 2:16 PM, "gfyoung" [email protected] wrote:

Is this a bug as well?

import numpy as np
mask = [True, False]
replace = 'FOO'

m = np.array(['A', 'B'])
np.place(m, mask, replace)
m
array(['F', 'O'],
dtype='|S1')

m = np.array(['A', 'B'])
np.copyto(m, replace, where=mask)
m
array(['F', 'O'],
dtype='|S1')

np.put(m, [0], replace)
m
array(['F', 'O'],
dtype='|S1')

It seems like strange things happen when the element size of the copied
elements in values / src differ from those in array / dest. In this case,
we get this unexpected truncation, which also occurs when you use integers
of different sizes (e.g. replace np.int8 with np.int64

—
Reply to this email directly or view it on GitHub
#6974 (comment).

@njsmith

@njsmith : I'm not too familiar with what the "regular casting rules" are. Are you saying that the code I showed above is to be expected because we're casting an '|S2' string into '|S1', which is the dtype of m?

I'm saying that place should do the same thing as, say, array assignment. Whatever that might be.

I guess assigning an S2 into an S1 does silently truncate, which is arguably a bad idea, but at least it means place and array assignment are consistent, so it's not a unique bug in place:

In [2]: a = np.array(["x", "y"])

In [3]: a[0] = "qq"

In [4]: a
Out[4]: 
array(['q', 'y'], 
      dtype='<U1')

I suspect if you try using these functions on record arrays then that might also produce some highly weird results...

@njsmith

@njsmith : Oh, no, I certainly agree it's not an issue just with np.place! That's why I demonstrated the issue with each of np.place, np.put, and np.copyto. Bug is everywhere! 😄

But okay, I understand what you mean though. In which case, the original issue that @WojciechMigda brought up is a "bug" in the sense that it is not consistent with what array assignment does, which is aligned with what np.putmask does.

My guess for the copyto case at least is that the string gets interpreted as an iterable. Not sure why this happens though, since array assignment (with `arr[...] = "Foo"``) does not do this, and it should use pretty much the same code IIRC.

Its possible that the indexing code somehow ensures conversion to array always (unless its an object array). The copyto code however might support any buffer, and a string is also a buffer.

@seberg

@seberg : I'll poke around and see.

@everyone: I've been trying to figure out how exactly that strange copying takes place with the example @WojciechMigda gave, and I think it's this line right here . It seems like the resulting element size in avals does not match that of ainput, even though its dtype information is passed in.

I tried changing the line to this to match the definition of PyArray_FromObject, except with the actual PyArray_DESCR(ainput) object passed in like this:

avals = (PyArrayObject *)PyArray_FromAny(vals, PyArray_DESCR(ainput), 0, 1,
                                           NPY_ARRAY_BEHAVED | NPY_ARRAY_ENSUREARRAY, NULL);

However, this segfaults Python due to a "STATUS_ACCESS_VIOLATION". Any ideas about why this is happening?

Navigation Menu

Repository navigation

numpy.place - strange behaviour with array of strings #6974

1 remaining item

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

numpy.place - strange behaviour with array of strings #6974

Description

Activity

gfyoung commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

seberg commented on Jan 12, 2016

njsmith commented on Jan 12, 2016

seberg commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

seberg commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

seberg commented on Jan 12, 2016

njsmith commented on Jan 12, 2016

seberg commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

njsmith commented on Jan 12, 2016

seberg commented on Jan 12, 2016

1 remaining item

seberg commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

njsmith commented on Jan 12, 2016

njsmith commented on Jan 12, 2016

gfyoung commented on Jan 12, 2016

njsmith commented on Jan 13, 2016

gfyoung commented on Jan 13, 2016

njsmith commented on Jan 13, 2016

gfyoung commented on Jan 13, 2016

seberg commented on Jan 13, 2016

seberg commented on Jan 13, 2016

gfyoung commented on Jan 13, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions