API, DEPR: Raise and Deprecate Reshape for Pandas Objects #13012

gfyoung · 2016-04-27T15:38:35Z

reshape largely exists for compat reasons with numpy but in the interests of moving away from that, this PR will cause objects like Series, Index, and Categorical to deprecate OR raise when such a method is called.

jreback · 2016-04-27T16:14:42Z

pandas/core/categorical.py

@@ -358,6 +358,19 @@ def itemsize(self):

    def reshape(self, new_shape, **kwargs):
        """ compat with .reshape """


can you make this a more propert doc-string (e.g. Parameters)

jreback · 2016-04-27T16:15:46Z

need these types of checks for Index as well. might as well combine the checks for Categorical,Index,Series (e.g. make a method to do it in core/base)

shoyer · 2016-04-27T17:11:26Z

This is definitely a good idea 👍

kawochen · 2016-04-27T17:56:30Z

had a glance. don't think you are handling (-2, -1)?

gfyoung · 2016-04-27T23:08:39Z

~~@jreback: Sure thing~~

@kawochen : Yikes, good catch. Will fix.

gfyoung · 2016-04-28T03:19:31Z

@jreback : Actually, on second thought:

Index has no reshape method --> but is this PR the right place to add such a method (is it really necessary even?)
Series may not need such checks IMO because it passes new_shape to its _values attribute for reshaping. AFAICT that _values attribute is either ndarray (or sub-class) OR Categorical. Seems like duplicate work to have it validate in Series and then have to validate again in _values. Thoughts?

jreback · 2016-04-28T13:48:59Z

Index should have one (that should just raise), as will Series in the next release. So this is just a bandaid. Categorical should raise as well, its an unsupported method.

gfyoung · 2016-04-28T13:55:47Z

The raising should be for a separate PR, right?
My point was that I saw no reason to perform validation twice given what _values can be. In light of your first point, I do agree that this is only a band-aid at best.

jreback · 2016-04-28T14:01:41Z

see my point above

gfyoung · 2016-04-28T14:05:20Z

Ah, okay, I see. Let them all raise then. Should be relatively straightforward.

jreback · 2016-04-28T14:09:02Z

@gfyoung but Series you can't change now as its technically an API change. however, would be ok with deprecating it now.

gfyoung · 2016-04-28T14:09:47Z

Shouldn't we do the same with Categorical then?

jreback · 2016-04-28T14:16:36Z

yes, didn't realize that .reshape was already defined.

gfyoung · 2016-04-28T14:22:03Z

Alrighty, so I'll deprecate for both Series and Categorical and add a method for Index that raises.

codecov-io · 2016-04-28T20:22:12Z

Current coverage is 84.38%

Merging #13012 into master will increase coverage by <.01%

@@             master     #13012   diff @@
==========================================
  Files           142        142          
  Lines         51225      51235    +10   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43225      43234     +9   
- Misses         8000       8001     +1   
  Partials          0          0

Powered by Codecov. Last updated by 20de266...3ad161d

gfyoung · 2016-04-29T00:32:17Z

@jreback : I've added deprecation and raising behaviour for Series, Index, and Categorical. Travis gives a green light, so this is ready to merge if there is nothing else.

jreback · 2016-04-29T13:27:26Z

let's do this in 0.19.0

jorisvandenbossche · 2016-07-11T09:32:23Z

pandas/core/categorical.py

+            by `new_shape` must be the same as that of the original
+            shape of the `Categorical`. One shape dimension can be
+            negative. In this case, the value is inferred from the
+            length of the array and the remaining dimensions.


Is it needed to add this documentation on new_shape if we are deprecating its usage?

yeah, would make these really simple.

As this is really a numpy thing, I'm just going to point the documentation on new_shape to np.reshape (we're deprecating this function after all, so I don't feel too bad for tying it to numpy).

jorisvandenbossche · 2016-07-11T09:39:31Z

@jreback What is the reason of adding a Index.reshape that raises? That seems unnecessary to me (and only adds to the list of Index methods).

For people using Series.reshape (not sure why they would do it, but probably it is used in the wild), is Series.values.reshape the alternative they should use instead?
If so, maybe add to the depr message?

jreback · 2016-07-11T10:03:06Z

@jorisvandenbossche we dont technically need this on Index as the idea is to raise on Series/Categorical in the future. (it should have been added originally to Index).

gfyoung · 2016-07-12T16:24:34Z

@jreback , @jorisvandenbossche : Made requested doc changes (no need to run Travis on those, hence the [ci skip]), so ready to merge if there are no other concerns.

jorisvandenbossche · 2016-07-12T17:51:00Z

@gfyoung See my comment from above: #13012 (comment)

gfyoung · 2016-07-12T17:54:25Z

@jorisvandenbossche: Ah whoops, good point. Will do.

jreback · 2016-07-13T02:18:45Z

pandas/core/internals.py

+                # and Series and will be removed in a subsequent version,
+                # hence the two checks done before calling 'reshape'
+                if isinstance(value, ABCSeries):
+                    value = value._values


what is this for?

oh I see.

ok, need an internal function to do this then (rather than having this duplicated code)

put in pandas.types.concat maybe compat_reshape

It was throwing warnings during testing.

The current code for Series.reshape will simply call self._values.reshape(...), so I am preempting that so we don't have to call the reshape method for Series.

and that's fine, except for have code duplication, so just put it in an internal function as I said above

I didn't get that second note until after I posted mine. 😄

Though why pandas.types.concat? We aren't really concatenating anything here. My first inclination was to try to put in pandas.compat, but I don't see a good home for it there.

u can also put in internals

Sounds good. Done.

jorisvandenbossche · 2016-07-13T09:02:52Z

pandas/indexes/base.py

@@ -944,6 +944,16 @@ def rename(self, name, inplace=False):
        """
        return self.set_names([name], inplace=inplace)

+    def reshape(self, *args, **kwargs):
+        """
+        Reshape an Index.


Can you put "Not implemented" on the first line? As otherwise in the methods overview in the Index docstring, you will see the reshape method with this explanation, and from that it would seem it has a working reshape method

Will do as soon as Travis gives a green light.

jreback · 2016-07-13T10:33:55Z

lgtm (minor doc comments). will need a rebase after #13147

gfyoung · 2016-07-14T04:17:52Z

@jreback : Made the requested doc changes as well as rebased, and Travis is still passing. Ready to merge if there are no other concerns.

jreback · 2016-07-14T10:21:38Z

thanks @gfyoung

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref pandas-devgh-13012

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref gh-13012

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref pandas-devgh-13012

jreback reviewed Apr 27, 2016
View reviewed changes

jreback added the Compat pandas objects compatability with Numpy or Python functions label Apr 27, 2016

jreback added this to the 0.18.1 milestone Apr 27, 2016

gfyoung force-pushed the categorical-reshape-validate branch 2 times, most recently from 208eaa8 to 2c7160e Compare April 28, 2016 08:15

jreback removed this from the 0.18.1 milestone Apr 28, 2016

gfyoung changed the title ~~API: Prevent invalid arguments to Categorical.reshape~~ API: Raise on Reshape for Pandas Objects Apr 28, 2016

gfyoung changed the title ~~API: Raise on Reshape for Pandas Objects~~ API, DEPR: Raise and Deprecate Reshape for Pandas Objects Apr 28, 2016

gfyoung force-pushed the categorical-reshape-validate branch from 2c7160e to 5498dc9 Compare April 28, 2016 20:22

jreback added this to the 0.19.0 milestone Apr 29, 2016

gfyoung force-pushed the categorical-reshape-validate branch 4 times, most recently from e77529e to e902d79 Compare April 30, 2016 21:41

jorisvandenbossche reviewed Jul 11, 2016
View reviewed changes

gfyoung force-pushed the categorical-reshape-validate branch from 24b6067 to 3860bc2 Compare July 11, 2016 13:22

gfyoung force-pushed the categorical-reshape-validate branch from 3860bc2 to 118d75f Compare July 13, 2016 02:06

jreback reviewed Jul 13, 2016
View reviewed changes

gfyoung force-pushed the categorical-reshape-validate branch from 118d75f to e849f9c Compare July 13, 2016 08:09

jorisvandenbossche reviewed Jul 13, 2016
View reviewed changes

gfyoung force-pushed the categorical-reshape-validate branch 3 times, most recently from 9c4e7a9 to 229060b Compare July 13, 2016 14:42

API: Prevent invalid arguments to Categorical.reshape

3ad161d

gfyoung force-pushed the categorical-reshape-validate branch from 229060b to 3ad161d Compare July 13, 2016 14:43

jreback closed this in 084ceae Jul 14, 2016

gfyoung deleted the categorical-reshape-validate branch July 14, 2016 14:52

gfyoung added a commit to forking-repos/pandas that referenced this pull request Dec 27, 2017

CLN: Drop the .reshape method from classes

c8b5d92

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref pandas-devgh-13012

gfyoung mentioned this pull request Dec 27, 2017

CLN: Drop the .reshape method from classes #18954

Merged

jreback mentioned this pull request Dec 27, 2017

DEPR: deprecations log for removed issues #13777

Closed

jreback pushed a commit that referenced this pull request Dec 27, 2017

CLN: Drop the .reshape method from classes (#18954)

e7d9102

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref gh-13012

hexgnu pushed a commit to hexgnu/pandas that referenced this pull request Dec 28, 2017

CLN: Drop the .reshape method from classes (pandas-dev#18954)

e248340

Remove the method for Series, Categorical, and Index. Deprecated or errored in v0.19.0 xref pandas-devgh-13012

		@@ -358,6 +358,19 @@ def itemsize(self):

		def reshape(self, new_shape, **kwargs):
		""" compat with .reshape """

API, DEPR: Raise and Deprecate Reshape for Pandas Objects #13012

API, DEPR: Raise and Deprecate Reshape for Pandas Objects #13012

Conversation

gfyoung commented Apr 27, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Apr 27, 2016

shoyer commented Apr 27, 2016

kawochen commented Apr 27, 2016

gfyoung commented Apr 27, 2016 • edited

gfyoung commented Apr 28, 2016 • edited

jreback commented Apr 28, 2016

gfyoung commented Apr 28, 2016 • edited

jreback commented Apr 28, 2016

gfyoung commented Apr 28, 2016 • edited

jreback commented Apr 28, 2016

gfyoung commented Apr 28, 2016

jreback commented Apr 28, 2016

gfyoung commented Apr 28, 2016

codecov-io commented Apr 28, 2016 • edited

Current coverage is 84.38%

gfyoung commented Apr 29, 2016

jreback commented Apr 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Jul 11, 2016 • edited

Choose a reason for hiding this comment

jorisvandenbossche commented Jul 11, 2016

jreback commented Jul 11, 2016

gfyoung commented Jul 12, 2016

jorisvandenbossche commented Jul 12, 2016

gfyoung commented Jul 12, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gfyoung Jul 13, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Jul 13, 2016

gfyoung commented Jul 14, 2016 • edited

jreback commented Jul 14, 2016

gfyoung commented Apr 27, 2016 •

edited

gfyoung commented Apr 27, 2016 •

edited

gfyoung commented Apr 28, 2016 •

edited

gfyoung commented Apr 28, 2016 •

edited

gfyoung commented Apr 28, 2016 •

edited

codecov-io commented Apr 28, 2016 •

edited

gfyoung Jul 11, 2016 •

edited

gfyoung Jul 13, 2016 •

edited

gfyoung commented Jul 14, 2016 •

edited