API: Rename CParserError to ParserError
State: closed

Title is self-explanatory.  Partially resolves #12665.


Labels:
name: Error Reporting
description: Incorrect or improved errors from pandas
name: IO CSV
description: read_csv, to_csv

Comments:
## [Current coverage](https://codecov.io/gh/pandas-dev/pandas/pull/14479?src=pr) is 85.29% (diff: 66.66%)

> Merging [#14479](https://codecov.io/gh/pandas-dev/pandas/pull/14479?src=pr) into [master](https://codecov.io/gh/pandas-dev/pandas/branch/master?src=pr) will increase coverage by **<.01%**

``` diff
@@             master     #14479   diff @@
==========================================
  Files           140        140          
  Lines         50706      50709     +3   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43247      43250     +3   
  Misses         7459       7459          
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [748000d...dceed5c](https://codecov.io/gh/pandas-dev/pandas/compare/748000d220c3c7d4daddf5811e10a5acdfce49ea...dceed5cc026f77ba2f11c98e80ee3198b44bf936?src=pr)

I think we should keep the old name as an alias (or at least deprecate it)

agreed this needs a deprecation.

can you also run the tests and be sure that we are not generating this in the full test run (I mean the deprecation warning).

@jreback : I can run them locally again, but I'm pretty sure the answer is no. :smile:

I would maybe go for a DeprecationWarning here in this case instead of FutureWarning, as this will be typically not used in interactive user code, but rather in library code (other option is to just keep it as alias)

@jorisvandenbossche : I'd rather deprecate since we would like to rename, though I don't quite see the necessity in making this one issue a `DeprecationWarning` instead of `FutureWarning`.

@jreback : Ran the tests locally and got no warnings about `CParserError`.

> I'd rather deprecate since we would like to rename, 

We did already rename it, my question was just whether to deprecate the old name or keep it as an alias, irregardless of the rename

> though I don't quite see the necessity in making this one issue a DeprecationWarning instead of FutureWarning.

This will typically be used in library code. People using such a library will see the FutureWarning, but cannot do anything about it (the warning will only go away once the library was updated), which is annoying. With a deprecation warning you only see it on direct use.

> People using such a library will see the FutureWarning, but cannot do anything about it (the warning will only go away once the library was updated), which is annoying. With a deprecation warning you only see it on direct use.

I fail to comprehend what you mean by that.

Putting `[ci skip]` in this commit until the failing `master` branch is patched.

@jreback , @jorisvandenbossche : Travis is now happy again.  Ready to merge if there is nothing else.

> > People using such a library will see the FutureWarning, but cannot do anything about it (the warning will only go away once the library was updated), which is annoying. With a deprecation warning you only see it on direct use.
> 
> I fail to comprehend what you mean by that.

By default, a DeprecationWarning is not visible, a FutureWarning is. Therefore, we mostly use FutureWarnings for deprecations in pandas, as most things (eg changing keyword) are things all users need to be aware of. However, in this case most users will not directly use CParserError, but rather use a third party library that uses it (this is an assumption of me, not sure if this is correct). In such a case, a DeprecationWarning can be friendlier to users, as they otherwise see FutureWarnings about CParserError being deprecated that they have no control over, as it is used in the library they used. For that reason, we also used DeprecationWarning for the `core.common` deprecations (https://github.com/pandas-dev/pandas/pull/13990)

Can you also add a test for the deprecated CParserError ?

@jorisvandenbossche : Ah, okay.  Done.  Let's see what Travis has to say now.

@jorisvandenbossche , @jreback : I changed the warning to be `DeprecationWarning`, and Travis seems to be happy.  Ready to merge if there are no other concerns.

lgtm. 

For me, this is not yet ready to be merged. The use of the decorator for deprecating CParserError breaks the usage in `try: .. except: ...`, which defeats the purpose of deprecating it

@jorisvandenbossche : I don't know of a way to issue a deprecation warning for `try-except`, especially if the exception actually never gets raised in the code.  Do you have any ideas then (i.e. of how to issue a deprecation warning just because someone tries to do `except CParserError`)?

Hmm, no not directly an idea (would maybe make a nice SO question :-)). As the exception is never 'called', you cannot really put it in the class definition itself I think. The way to get a warning, would be to get a warning when importing it. But since it is not a module, also not that easy to do I think. 

Personally I would say, let's just leave it as an alias if we do not see a way to deprecate it.

@jorisvandenbossche : That sounds good to me.  I'll just alias and add a comment to remove later.

@jorisvandenbossche , @jreback : I aliased `ParserError` to `CParserError` with a note to remove in the future.  Ready to merge if there is nothing else.

Getting some weird segfaults that seem unrelated to this PR (also in my other one)...disabling testing for the time being.

@jorisvandenbossche , @jreback : No more segfaulting, and everything looks green.  Ready to merge if there are no concerns.

I would first like to get agreement on the status of CParserError: do we regard it as an alias (which means for me keeping it), or as deprecated and with the intention to remove it?

I asked SO if it possible to deprecate an Exception (http://stackoverflow.com/questions/40545005/is-it-possible-to-deprecate-an-exception-class/40546615#40546615), but there did not really appear a nice solution

@jorisvandenbossche : The intention is to remove `CParserError` as both I and @jreback have said (see <a href="https://github.com/pandas-dev/pandas/issues/12665#issuecomment-211044839">here</a> for his comment regarding this).  However, if there is no nice way to deprecate an  exception, then aliasing as a preliminary step is the only way to go at this point.

@jorisvandenbossche : Added a slightly different alias test, but yes, done.

@gfyoung Thanks! (just added the same test for pd.parser.CParserError)