NotImplementedError messages doc update #7872
State: closed

closes #7872

I have updated the NonImplementedError messages #7872 in all places applicable.
It includes changes to statements of type
- raise NotImplementedError
- raise NotImplementedError()
  Comment on the changes. 

The one place I have doubt and not sure whether to add one or not would be https://github.com/pydata/pandas/blob/master/.coveragerc#L17


Labels:
name: Docs
name: Error Reporting
description: Incorrect or improved errors from pandas

Comments:
don't worry about the coverage file

pls squash the commits to one, see here: https://github.com/pydata/pandas/wiki/Using-Git

@hayd @cpcloud @jorisvandenbossche 

usually for an abstract type class I would NotImplementedError; is this standard? anything else to indicate that its an abc method?

there's the `abc` module:

``` python
from abc import abstractmethod, ABCMeta


class CannotInstantiate(object):
    __metaclass__ = ABCMeta

    @abstractmethod
    def hardy_har(self):
        pass
```

This will raise `TypeError: Can't instantiate abstract class CannotInstantiate with abstract methods hardy_har`
What's a little annoying is that you can't use `abstractmethod` without using the `ABCMeta` as a metaclass _and_ Python 3 has different and incompatible syntax for specifying a metaclass

:rage:

ok maybe should then have the not implemented errors signify that they are abstract 

e.g. in indexing, the `_LocationIndexer` is an abstract class. something like "NotImplemenetedError('this is an abstract class ......")`` mainly to signal to a sub-class implementer. like a convention.

or the message like you did above

All the commits are squashed into one commit. Next time, I will careful to do the same before pushing it to the public repo.

Thanks for going through this! Part of the reason for the original issue is that the error message is it should say _why_ not just where/what.... Unfortunately, I can't find any specific advice online about writing good exceptions messages. :( I do think these ought to be proper sentences...

IMO it would be nice if abstract methods were reflected in the error message (confession: I also do this)... Would it be easier/compat solution to define an AbstractMethodError <: NotImplementedError (whose message is (or is prefixed with) "This method must be defined on the concrete class" or something)? :s

@hayd thats a great idea!

@ilam want to see if you can define (prob in core/common.py) and change ones that are Abstract classes (i'll look thru after)

@jreback Sure. As far as I understand, instead of raising NotImplementedError, instead raising AbstractMethodError (after defining it) with the appropriate message would be good. Right?

@ilam exactly. If you have about whether certain classes should be abstract or not, pls post a link here and we can discuss.

@jreback Is this the right way of doing it? ~~8e5ca6f~~. Updated ae8b15c

@jreback Sure, I am sorry, I made that mistake again, I will squash them, and include here a list of Abstract classes, if any for which these changes might be required.

https://github.com/pydata/pandas/blob/master/pandas/core/indexing.py#L58
Classes `_LocationIndexer`, `_NDFrameIndexer`, `_iLocIndexer` are all abstract classes and have the `NotImplementedError` in them.

yep

Cool. Will change them to `AbstractMethodError` in those abstract classes.

@cpcloud solution for the python 2/3 meta class thing is to copy six's with_metaclass into compat http://stackoverflow.com/a/18513858/1240268 using `@abstractmethod` would be the best solution!

@ilam want to rebase & update?

@ilam can you rebase and revisit this?

replaced by #9887