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

Single component value can not be changed in VectorNumpy3D #158

Open
cansik opened this issue Dec 9, 2021 · 2 comments
Open

Single component value can not be changed in VectorNumpy3D #158

cansik opened this issue Dec 9, 2021 · 2 comments
Labels
feature New feature or request

Comments

@cansik
Copy link
Contributor

cansik commented Dec 9, 2021

I am using the vector library in a computer-vision software to store the prediction results. There I have a two-step workflow where I first have to calculate some values, then refine a part of them (for example the depth component z). After implementing everything I never got the right results and wondered why.

I noticed that it is possible to set a single component in a vector-object:

v = vector.obj(x=1.1, y=2.2)
v.x = 25

print(v)
# vector.obj(x=25, y=2.2)

But this is not possible as soon as I work with VectorNumpy3D (and 2D, 4D):

v = vector.array({
    "x": [1.0, 2.0],
    "y": [1.1, 2.2],
    "z": [0.1, 0.2],
})

v[1].x = 25

print(v)
# [(1., 1.1, 0.1) (2., 2.2, 0.2)]

Either there should be an error message, that it is not possible to set single component values in VectorNumpy3D arrays or the value should be set. Atm the line of code seems just like being ignored.

Is there a way to change a single value inside of this array structure?

@cansik cansik changed the title Single component value can not be changed in VectorNumpy3D. Single component value can not be changed in VectorNumpy3D Dec 9, 2021
@jpivarski
Copy link
Member

Hmm. It's hard to see how there can be a way to do what you want (either an error message or a change in the original array). When you say v[1], you get a new VectorObject3D, and you can set the x value of that new object to 25, then that new object goes out of scope. Pandas has this problem too, and they raise a SettingWithCopyWarning (which is complained about far more than any other Pandas warning).

If, instead of

v[1].x = 25

you had said

v.x[1] = 25

it would have assigned to the original array: v.x makes a NumPy array that shares memory with the vector array, so when you assign to that, you get the change in the original.

Mutability and views vs copies have always been murky, which is why there's such a strong movement behind functional programming.

Anyway, to actually implement this, we would need a new parameter to VectorObject*Ds or a new subclass that links it to the original array, so that we know how to propagate changes back. However, that would have the surprising consequence that extracting some elements from an array and deleting the array won't actually free any memory because each extracted element holds a reference to the whole array. VectorObject*Ds that come from Awkward Arrays can't have this feature because Awkward Arrays are immutable (the view-vs-copy issue is even more complex for them). Maybe instead, there should be a parameter or new type of VectorObject*D that is not connected to the original array but has a "do not write" flag, so that at least you get an error message, whether the VectorObject*D is derived from a NumPy array or Awkward Array. However, that prohibition against writing is merely formal: some users would want to write to these objects and they'd need a way to override the "do not write" flag, which would be another flag.

At the end of that brainstorming, the "do not write" flag sounds like it would cause the least problems, and it would present an error message explaining all of these issues with a way to override the flag. I suppose that would be a writable: bool property on VectorObject*D. What do you think?

(I'm going to label this as a feature request.)

@jpivarski jpivarski added the feature New feature or request label Dec 9, 2021
@cansik
Copy link
Contributor Author

cansik commented Jan 6, 2022

@jpivarski Thank you very much for the comprehensive explanation.
I guess for now it's fine to access the original array with v.x[1] = 25.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants