BUG: Series.reshape to own shape raises TypeError

[goyodiaz]

closes #4554

I just fixed the call to ndarray.reshape with order as a positional argument, which is wrong. Please note that I had to remove a test.

[jreback]

• can you add release notes entry
• hook up travis (see contributing.md)

[hayd]

maybe use assert_series_equal.

How comes you deleted the test above? Is there a reason not just do the same test there (i.e. make it assert_series_equal(x.reshape(len(x),), x)? Does this work with name (assert_series_equal will test name too)?

[jreback]

ahh..but the reshape actually returns an ndarray here (which IMO) is a bit odd, but its for numpy compat;

@goyodiaz add an assertion about the type as well

[hayd]

@jreback If this is all for numpy compat, why can't we just do arbitrary reshapes? I may be asking a silly question here...

[jreback]

no, on an exactly equal reshape you get back the series, anything else you get a numpy array (assume its equal sized), but the ndim has changed, e.g. (2,2) for a len(4) series is ok, but by definition you get a numpy array; I think mainly for compat?

[hayd]

Doesn't this mean the test above will fail? (i.e. not raise)

[jreback]

no, because it compares a with a (both are series), which is correct (though this should be)
assert_series_equal though, @goyodiaz. I know the API is a little screwy, but ndarrays permit ndim changes, while pandas does not, so not much choice, any better ideas?

[hayd]

@jreback Sorry, I meant this one: self.assertRaises(TypeError, a.reshape, 2, 2)

[jreback]

that looks a.reshape((2,2)) should just return an ndarray I think

[goyodiaz]

@hayd Point taken, I'll try to minimize changes to existing tests.
@jreback Well, there is some choice. We could (a) just reshape the values and always return an ndarray or (b) always return a pandas object if/when pandas can deal with it. I actually like (a).

Back to the code, I realized that more work needed to be done. There are currently two assertions in test_reshape_non2d():

x = Series(np.random.random(201), name='x')
self.assertRaises(TypeError, x.reshape, (len(x),))

This is the bug I reported. x.reshape((len(x),)) should not raise TypeError but return x itself. But I think x.reshape(s.shape) makes more sense in this context. I can live with the 201 and the random numbers.

# GH 2719
a = Series([1, 2, 3, 4])
self.assertRaises(TypeError, a.reshape, 2, 2)

a.reshape(2, 2) should return a.values.reshape(2, 2) without raising any exception. Asserting this also prevents against #2719.

So I have changed the test according to this and rewrite Series.reshape() to pass it. I had to change the signature to make it accept the same input as ndarray.reshape().

Sorry I got git to make a mess of my fork's history. I have been told this can be cleaned up but I am not used to git so I would appreciate some advice.

[jtratner]

To cleanup your branch run the following

git remote add upstream https://www.github.com/pydata/pandas.git
git fetch upstream
# will need to fix merge conflicts here
git rebase upstream/master
# pops up window, change 'pick' to 'squash' to combine commits
git rebase -i upstream/master 

[cpcloud]

fyi ... you can also replace "pick" with "fixup" to remove the commit messages that squash leaves hanging around. this is great for those commits that fix typos/syntax errors, etc.

[jreback]

@cpcloud anything else?

[cpcloud]

@goyodiaz Needs to be rebased.

[goyodiaz]

Sorry, not sure what I'm expected to do. I rebased my changes on top of upstream again.

[cpcloud]

probably a merge conflict with release notes .... gotta do it again

[cpcloud]

travis is passing so that's good ... just resolve the merge conflict and BAM! it'll get merged :)

[goyodiaz]

One of the travis build jobs failed but I don't understand how it can be related to my patch.

[jtratner]

It's not crazy that a change to reshape might have caused an issue, though it does seem unrelated. @jreback - thoughts on the failing build?

[jreback]

nope...that's a ocassional test failure that have never been able to nail down (travis once in a while fails it, but never can reproduce locally)....I think this is ready to merge, any objections?

[jreback]

@goyodiaz thank you!