Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PanicException doesn't inherit from base Exception #5606

Closed
2 tasks done
david-cortes opened this issue Nov 23, 2022 · 7 comments
Closed
2 tasks done

PanicException doesn't inherit from base Exception #5606

david-cortes opened this issue Nov 23, 2022 · 7 comments
Labels
bug Something isn't working python Related to Python Polars

Comments

@david-cortes
Copy link

Polars version checks

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

  • 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 david-cortes added bug Something isn't working python Related to Python Polars labels Nov 23, 2022
@ritchie46
Copy link
Member

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.

@david-cortes
Copy link
Author

david-cortes commented Nov 25, 2022

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.

@davidhewitt
Copy link

davidhewitt commented Nov 26, 2022

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 PyO3/pyo3#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?

@david-cortes
Copy link
Author

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.

@mkleinbort-ic
Copy link

mkleinbort-ic commented Nov 30, 2022

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

@ghuls
Copy link
Collaborator

ghuls commented Dec 7, 2022

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"))

@ritchie46
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working python Related to Python Polars
Projects
None yet
Development

No branches or pull requests

5 participants