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

Masked WeightedSumView returns copy? #371

Open
HDembinski opened this issue Jun 4, 2020 · 3 comments
Open

Masked WeightedSumView returns copy? #371

HDembinski opened this issue Jun 4, 2020 · 3 comments
Labels
docs Documentation

Comments

@HDembinski
Copy link
Member

HDembinski commented Jun 4, 2020

I tried to assign to a WeightedSumView. It works correctly, when the view is not masked, but if it is masked, it does not work. Pseudo code:

v = some_weighted_hist.view()
v.value[:] = [1, 2, 3] # work ok, v.value is now [1, 2, 3]
v[v.value == 2].value[:] = [3] # does not work, v.value is still [1, 2, 3]

I suppose that the masking operation returns a copy instead of a view.

@HDembinski
Copy link
Member Author

Follow-up: this works

v.value[v.value == 2] = [3] # ok, v.value is now [1, 3, 3]

I guess this kind of behavior makes sense. The second version calls __setitem__ with the mask, so it knows which cells to update, while the earlier version above first generates a new copy from the mask on which I then call __setitem__.

@henryiii
Copy link
Member

henryiii commented Jun 4, 2020

I wrote a long reply about histograms not supporting masks before realizing you were operating on a view. Yes, this is part of the Python syntax. Slices do not copy, because of the stride and offset arrays in NumPy (or Python MemoryViews, or Eigen, etc...). But selections like the one above do copy; they have to, since stride and offset cannot perform completely arbitrary selections. So in example 1 you select and then set, while in 2 you select and set in the same operation.

@HDembinski
Copy link
Member Author

Thank you for confirming, so this is not a bug, but this somewhat surprising behavior probably should be documented.

@henryiii henryiii added the docs Documentation label Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation
Projects
None yet
Development

No branches or pull requests

2 participants