polars icon indicating copy to clipboard operation
polars copied to clipboard

`PanicException` doesn't inherit from base `Exception`

Open david-cortes opened this issue 2 years ago • 6 comments

Polars version checks

  • [X] I have checked that this issue has not already been reported.

  • [X] I have confirmed this bug exists on the latest version of Polars.

Issue description

polars tends to throw PanicException for user-caused errors, such as supplying invalid column names or similar. This exception however doesn't inherit from Python's base Exception, which makes it problematic to catch errors.

Reproducible example

import polars as pl
print(isinstance(pl.PanicException(), Exception))
print(isinstance(pl.PanicException(), BaseException))

Expected behavior

Should inherit from Exception, as BaseException is reserved for special errors like KeyboardInterrupt and one usually wants to catch things derived from Exception but not from BaseException.

Installed versions

---Version info---
Polars: 0.14.31
Index type: UInt32
Platform: Linux-5.18.0-4-amd64-x86_64-with-glibc2.36
Python: 3.9.12 (main, Apr  5 2022, 06:56:58) 
[GCC 7.5.0]
---Optional dependencies---
pyarrow: 8.0.0
pandas: 1.4.2
numpy: 1.21.5
fsspec: 2022.02.0
connectorx: <not installed>
xlsx2csv: <not installed>
matplotlib: 3.5.1

david-cortes avatar Nov 23 '22 16:11 david-cortes

I think this discussion should be upstream in pyo3.

https://pyo3.rs/main/doc/pyo3/panic/struct.panicexception

A panic should not be handled as something that can be caught. We should treat it as a bug in polars or a severe problem in your code we cannot recover from.

ritchie46 avatar Nov 25 '22 07:11 ritchie46

Ok, will open an issue in pyo3 then.

However, I am not so sure that PanicException as used by polars is something that should be interpreted as an irrecoverable error. For example, if you are running a service that does some aggregation on some data and returns a number, and such operation fails due to e.g. supplying an invalid column name, it could be reasonable to make the service output something like "NaN" or zero instead of throwing an error. Catching except Exception allows that, while still making the service responsive to interrupt signals for example.

david-cortes avatar Nov 25 '22 17:11 david-cortes

A panic should not be handled as something that can be caught. We should treat it as a bug in polars or a severe problem in your code we cannot recover from.

As per the comments in https://github.com/PyO3/pyo3/issues/2783 it sounds like both polars and PyO3 authors are in agreement that panics are bugs in libraries rather than something users are expected to catch.

@david-cortes perhaps you can provide examples of panics (maybe as separate issues) so that the polars team can investigate and fix them?

davidhewitt avatar Nov 26 '22 15:11 davidhewitt

Example:

import numpy as np, polars as pl
(
    pl.DataFrame({"col1" : np.arange(10)})
    .lazy()
    .select("col2")
    .rename({"col2" : "col3"})
)

or the example from the other thread linked here:

pl.DataFrame({'x':['Some text']}) / 0

Panics happen a lot when dealing with lazy frame operations.

david-cortes avatar Nov 28 '22 15:11 david-cortes

Just to illustate how catching a panic exception would be good:

try: 
    # {fast polars implementation}
except Exception as e:
    logger.warning(f'Using slow implementation as a result of {e}')
    df_tmp = df.to_pandad()
    # {slow pandas implementation}
    df = df_ans.pipe(pl.from_pandas)

I often want to catch generic things (if only so I can try something else), but not keyboard interrupts

mkleinbort-ic avatar Nov 30 '22 10:11 mkleinbort-ic

You can catch panic exceptions specificly, if you really need that option:

import numpy as np
import polars as pl

In [9]: try:
   ...:     
   ...:     (
   ...:     pl.DataFrame({"col1" : np.arange(10)})
   ...:     .lazy()
   ...:     .select("col2")
   ...:     .rename({"col2" : "col3"})
   ...:     )
   ...: except pl.PanicException as pe:
   ...:     print("Catched PanicException:", pe)
   ...: 
thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: NotFound(Owned("col2"))', /home/luna.kuleuven.be/u0079808/software/polars/polars/polars-lazy/src/frame/mod.rs:418:40
Catched PanicException: called `Result::unwrap()` on an `Err` value: NotFound(Owned("col2"))

ghuls avatar Dec 07 '22 13:12 ghuls

I will close this as I think we have explained the situation.

ritchie46 avatar Dec 30 '22 09:12 ritchie46