DOC: Improve ndarray.shape documentation. #9810

charris · 2017-10-02T20:50:22Z

Update of #9571.

[ci skip]

eric-wieser · 2017-10-03T07:19:29Z

numpy/add_newdocs.py

-    require a change in the total number of elements
+    The shape attribute is usually used to get at the current shape of an
+    array, but may also be used to reshape the array by assigning a different
+    tuple to it.  As with `numpy.reshape`, one of the new shape dimensions can


'shape entries' or 'array dimensions' would be clearer

eric-wieser · 2017-10-03T07:20:14Z

numpy/add_newdocs.py

+    array, but may also be used to reshape the array by assigning a different
+    tuple to it.  As with `numpy.reshape`, one of the new shape dimensions can
+    be -1, in which case its value is inferred from the length of the array and
+    remaining dimensions. This method of reshaping an array is not recommended,


The sentence ordering here might lead the reader to think that this sentence refers simply to the -1 behaviour.

eric-wieser · 2017-10-03T07:21:14Z

numpy/add_newdocs.py

+    tuple to it.  As with `numpy.reshape`, one of the new shape dimensions can
+    be -1, in which case its value is inferred from the length of the array and
+    remaining dimensions. This method of reshaping an array is not recommended,
+    both because it can fail, and because it affects any code holding a


This is actually a feature I hadn't considered. For instance:

def reshape_view_only(x, shape): a = array(x. copy=False) a.shape = shape return a

eric-wieser · 2017-10-03T07:22:05Z

numpy/add_newdocs.py

-    May be used to "reshape" the array, as long as this would not
-    require a change in the total number of elements
+    The shape attribute is usually used to get at the current shape of an
+    array, but may also be used to reshape the array by assigning a different


Maybe add "in place" here?

charris · 2017-10-03T17:28:52Z

Fixed.

eric-wieser · 2017-10-03T17:52:08Z

numpy/add_newdocs.py

+    array dimensions to it.  As with `numpy.reshape`, one of the new shape
+    dimensions can be -1, in which case its value is inferred from the length
+    of the array and remaining dimensions. Reshaping an array in-place is not
+    recommended; not only may it fail, but it also affects any code holding a


I think elaborating on may it fail would be good, since I think that is the one reason to use it. Perhaps Unlike ndarray.reshape, this throws an exception if the reshape would require a copy.

shoyer · 2017-10-03T18:59:59Z

numpy/add_newdocs.py

+    array dimensions to it.  As with `numpy.reshape`, one of the new shape
+    dimensions can be -1, in which case its value is inferred from the length
+    of the array and remaining dimensions. Reshaping an array in-place will
+    fail if a copy is required, and it affects any code holding a reference to


"affects any code holding a reference to the array in an indirect way" almost suggests that it affects views of the same array data, which is not the case.

So I'm not sure this part is helpful. It's really a statement about how creating new objects should be preferred to mutation (which I generally agree with), but this sort of general coding advice feels slightly out of place in a doc-string.

I wanted to discourage using it for reshaping. Long ago I was tempted make a PR making ndarray.shape readonly, but backwards compatibility was a problem. Then again, I would rather ndarray.reshape raised an error if a copy was needed. Oh well ;)

Hmm. how come owndata if false here when a copy is made?

In [18]: ones((4,2))[::2].reshape(4).flags Out[18]: C_CONTIGUOUS : True F_CONTIGUOUS : True OWNDATA : False WRITEABLE : True ALIGNED : True UPDATEIFCOPY : False

Removed the comment about the mutation.

Then again, I would rather ndarray.reshape raised an error if a copy was needed.

Exactly. This is one valid reason for mutating .shape. See #9818 for my proposal for adding a copy argument to reshape().

eric-wieser · 2017-10-04T05:46:32Z

numpy/add_newdocs.py

+    The shape property is usually used to get the current shape of an array,
+    but may also be used to reshape the array in-place by assigning a tuple of
+    array dimensions to it.  As with `numpy.reshape`, one of the new shape
+    dimensions can be -1, in which case its value is inferred from the length


nit: size of the array, not length

eric-wieser · 2017-10-04T05:51:11Z

numpy/add_newdocs.py

@@ -2949,6 +2951,11 @@ def luf(lamdaexpr, *args, **kwargs):
      File "<stdin>", line 1, in <module>
    ValueError: total size of new array must be unchanged


Can we add an example here for reshaping a non-contiguous array? Something a little less contrived than:

>>> np.eye(3)[:2,:2].shape = (-1,) AttributeError: incompatible shape for a non-contiguous array

With or without error?

With error. We already have an example of a without-error case. Should have said "non-contiguous array with an incompatible shape".

All non-contiguos examples are a bit contrived. Here's another

>>> np.zeros((4,2))[::2].shape = (-1,) Traceback (most recent call last): File "<stdin>", line 1, in <module> AttributeError: incompatible shape for a non-contiguous array

Changing the error message is a bit out of scope, but the current one seems OK to me, it is the (-1,) that is the incompatible shape.

[ci skip]

charris · 2017-10-04T18:38:27Z

Added example of failure.

DOC: Improve ndarray.shape property documentation.

db59c45

[ci skip]

charris added 04 - Documentation component: numpy._core labels Oct 2, 2017

charris mentioned this pull request Oct 2, 2017

DOC: added info on shape property #9571

Closed

charris force-pushed the gh-9571 branch 2 times, most recently from 65dfd03 to 3837985 Compare October 2, 2017 20:55

eric-wieser reviewed Oct 3, 2017

View reviewed changes

charris force-pushed the gh-9571 branch from 3837985 to a05f8cb Compare October 3, 2017 17:28

eric-wieser reviewed Oct 3, 2017

View reviewed changes

charris force-pushed the gh-9571 branch 4 times, most recently from 00582e8 to 9cfc01c Compare October 3, 2017 18:28

shoyer reviewed Oct 3, 2017

View reviewed changes

charris force-pushed the gh-9571 branch from 9cfc01c to 95e6b8e Compare October 3, 2017 19:46

shoyer mentioned this pull request Oct 3, 2017

Proposal: add an optional copy argument to np.reshape #9818

Open

eric-wieser reviewed Oct 4, 2017

View reviewed changes

charris force-pushed the gh-9571 branch from 95e6b8e to a10de60 Compare October 4, 2017 14:55

DOC: Update ndarray.shape property documention.

6a5e618

[ci skip]

charris force-pushed the gh-9571 branch from a10de60 to 6a5e618 Compare October 4, 2017 18:26

eric-wieser approved these changes Oct 5, 2017

View reviewed changes

charris merged commit 804cb0e into numpy:master Oct 5, 2017

charris deleted the gh-9571 branch October 5, 2017 13:44

eric-wieser mentioned this pull request Dec 12, 2017

DOC: one dimension can be -1 works also for numpy.ndarray.shape. #9539

Closed

		@@ -2949,6 +2951,11 @@ def luf(lamdaexpr, args, *kwargs):
		File "<stdin>", line 1, in <module>
		ValueError: total size of new array must be unchanged

Uh oh!

DOC: Improve ndarray.shape documentation. #9810

DOC: Improve ndarray.shape documentation. #9810

Uh oh!

Conversation

charris commented Oct 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris commented Oct 3, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris Oct 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eric-wieser Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charris commented Oct 4, 2017

Uh oh!

Uh oh!

charris Oct 3, 2017 •

edited

Loading

eric-wieser Oct 4, 2017 •

edited

Loading