You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Can I attempt to complete this task ? I am not completely accustomed to the core of numpy but it would be nice if I can try to go through the code for understanding a bit of core python too, since I think this is the line that needs to be removed for the view to be read/write ( as commented in code) https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/item_selection.c#L2193
I had found this by searching for PyArray_Diagonal using grep. So, it would be helpful if someone would guide how to find the corresponding core function , say ravel, diagonal or clip.
The change needs to be in the assert statement of the a.flags.writeable flag, which needs to be inverted, but the change for the a.flags.owndata need not be changed, I suppose. Do correct me if I am wrong.
@charris, I wanted to try out the new changes proposed, it looks like the PyArray_Diagonal needs to be reverted to the scenario before 1.9.0, where a copy was created and returned.here. Though there was a FutureWarning raised then, I think it should be a different scenario now.
Also, it would be great if you could suggest whether implementing this whole task is feasible for a newcomer like me. If yes, I would want to give it a try to implement the new feature.
Gently bumping the issue. I assume the issue has been indeed moved to 1.11 but the docstring needs some updating in that case. (I would also reorder the sections of the docstring so that "current behavior" rather than "past behavior" goes to the top.)
Moving to 1.12, since we haven't talked about this and 1.11 is imminent. Docstrings have been fixed in master to state that this is a planned-at-some-indefinite-future-date rather than giving a specific version.
Seems like it would be a nice feature some day. Especially since it was already documented as a feature. What's the "community debate wrangling" here? Should I even ask? ;)
IIRC, there is a failing test. The transition was from returning a copy of the diagonal to returning a view, that's why the initial step was to make the view ro so folks would not accidentally change the original array.
What's the "community debate wrangling" here? Should I even ask? ;)
It's good to ask. There was a long debate on how bad the backwards compatibility break was: http://article.gmane.org/gmane.comp.python.numeric.general/59620. In hindsight we probably never should have made any changes. It's still a bit up in the air what we'll do now and when I think.
Activity
maniteja123 commentedon Jan 1, 2015
Can I attempt to complete this task ? I am not completely accustomed to the core of numpy but it would be nice if I can try to go through the code for understanding a bit of core python too, since I think this is the line that needs to be removed for the view to be read/write ( as commented in code)
https://github.com/numpy/numpy/blob/master/numpy/core/src/multiarray/item_selection.c#L2193
I had found this by searching for
PyArray_Diagonalusinggrep. So, it would be helpful if someone would guide how to find the corresponding core function , sayravel,diagonalorclip.charris commentedon Jan 1, 2015
Sure, have at it. Looks like you found the right spot. There should be a test somewhere that also needs to be updated. If not, you can add one :)
maniteja123 commentedon Jan 1, 2015
@charris Thanks :) I will also check if there is corresponding test regarding the
diagonalcommand.maniteja123 commentedon Jan 1, 2015
I think this is the test to check the writability of the view returned by the
diagonal. https://github.com/numpy/numpy/blob/master/numpy/core/tests/test_multiarray.py#L1738The change needs to be in the
assertstatement of thea.flags.writeableflag, which needs to be inverted, but the change for thea.flags.owndataneed not be changed, I suppose. Do correct me if I am wrong.charris commentedon Jan 1, 2015
Yep, that is the place and the changes you suggest look right. Don't forget to rename the test function.
maniteja123 commentedon Jan 1, 2015
Yeah, sure. When we change the name of the test function, do we need to update anywhere ?
charris commentedon Jan 1, 2015
Should not matter, nose picks up the test functions by their prefix
test.maniteja123 commentedon Jan 1, 2015
Oh thanks , just wanted to confirm. I referenced it in the PR #5409 .
maniteja123 commentedon Jan 16, 2015
@charris, I wanted to try out the new changes proposed, it looks like the
PyArray_Diagonalneeds to be reverted to the scenario before 1.9.0, where a copy was created and returned.here. Though there was aFutureWarningraised then, I think it should be a different scenario now.Also, it would be great if you could suggest whether implementing this whole task is feasible for a newcomer like me. If yes, I would want to give it a try to implement the new feature.
njsmith commentedon Jan 16, 2015
@maniteja123: Honestly, I'd stay clear of this one as a newcomer -- it's ~5% programming and 95% community debate wrangling.
maniteja123 commentedon Jan 16, 2015
@njsmith I do understand the implications of this feature. I just wanted to clarify. Thanks :)
anntzer commentedon Sep 28, 2015
Gently bumping the issue. I assume the issue has been indeed moved to 1.11 but the docstring needs some updating in that case. (I would also reorder the sections of the docstring so that "current behavior" rather than "past behavior" goes to the top.)
njsmith commentedon Jan 15, 2016
Moving to 1.12, since we haven't talked about this and 1.11 is imminent. Docstrings have been fixed in master to state that this is a planned-at-some-indefinite-future-date rather than giving a specific version.
jakirkham commentedon Jan 16, 2016
Seems like it would be a nice feature some day. Especially since it was already documented as a feature. What's the "community debate wrangling" here? Should I even ask? ;)
charris commentedon Jan 17, 2016
IIRC, there is a failing test. The transition was from returning a copy of the diagonal to returning a view, that's why the initial step was to make the view ro so folks would not accidentally change the original array.
rgommers commentedon Jan 17, 2016
It's good to ask. There was a long debate on how bad the backwards compatibility break was: http://article.gmane.org/gmane.comp.python.numeric.general/59620. In hindsight we probably never should have made any changes. It's still a bit up in the air what we'll do now and when I think.