CLN: Replace bare exceptions with more descriptive ones

[jtratner]

I'm moving this to an issue so it's easier to track and I can split into separate, smaller, pull requests. (previously was #3924).

For right now, the goal is to take care of the low-hanging fruit (the really obvious items that should be TypeError, ValueError, etc.) and then potentially take on more of the code base later.

Packages/Files where exceptions have been changed (will be checked off once test cases are edited for them)

• merge
• stats/common
• tools/tile
• core/common
• core/format
• core/frame
• pandas/sparse
• pandas/tseries
• core/internals
• core/index
• core/panel
• core/series
• core/panelnd
• rpy/common
• io/excel

More to come...

[jtratner]

Btw all, I was thinking for guidelinesihat any exception that's a marker (e.g. In groupby when deciding how to aggregate / combine, or in other places) and only going to be internal, can use a custom exception that subclasses PandasError. (if it's necessary to have some kind of sentinel for that) and in general anything that would bubble up to the user should try to use builtin exceptions where possible. Going forward, no test case should use bare assertRaises (Exception...) and overall goal is to remove all places that raise bare Exception. Good example of the problem with that method of testing are the test for _cached_range which were passing wrong keyword args which caused TypeError s . If it makes sense encouraged to use assertRaisesRegexp which I back ported from 2.7 unittest. (makes it easier to see raising the right error). Does this seem reasonable? @jreback @cpcloud?

[cpcloud]

just saw this. sounds good to me.

[jreback]

related to #4083

[jtratner]

@jreback should this be an error?

DataFrame([[1,2,'foo','bar']], columns=['a','a','a','a'])

The test case for this was raising an exception because it expected an argument, so I'm not clear if it ought to or not...

# in actuality, this is an error because it calls `lambda x:` with zero arguments...
# this is an error because we cannot disambiguate the dup columns
self.assertRaises(Exception, lambda x: DataFrame([[1,2,'foo','bar']], columns=['a','a','a','a']))

[jtratner]

@jreback similar question about this one:

        ## where...
        self.ts1 = tm.makeTimeSeries()
        self.ts2 = tm.makeTimeSeries()[5:]

        #corner cases
        self.assertRaises(Exception, lambda x: DataFrame([self.ts1, self.ts2]))

(DataFrame([self.ts1, self.ts2])) works in current master

[hayd]

Neither of those make sense anymore even if they were corrected, since as you say both work in master. I think delete them or just have them be the direct call (and comment to say they used to raise).

[jtratner]

@hayd thanks - that's helpful! (I was looking for a second opinion here).

[jtratner]

@hayd fun error! - shouldn't irow(0) here result in something like Series([1, 2, foo, bar], index=['a', 'a', 'a', 'a'], name=0)?

In [3]: from pandas import *
In [4]: df = DataFrame([[1,2,'foo','bar']], columns=['a','a','a','a'])
In [5]: df
Out[5]:
   a  a    a    a
0  1  2  foo  bar

In [6]: df.irow(0)
Out[6]:
a    bar
a    bar
a    bar
a    bar
Name: 0, dtype: object

In [7]: df.icol(0)
Out[7]:
0    1
Name: a, dtype: int64

In [8]: df.columns
Out[8]: Index([u'a', u'a', u'a', u'a'], dtype=object)

[jtratner]

suggests to me that duplicate columns should be disallowed (and obv hierarchical index special cased)

[hayd]

Hmmmm, I think this should work as you say, is definitely a bug. Interesting dupe index works so can do df.T.iloc[:, 0].

Don't think should disallow dupe cols...

[jtratner]

See #4726