Python API: log and swallow all exceptions #549
Conversation
rerun_py/rerun/__init__.py
Outdated
| # Add this to ALL user-facing functions! | ||
| # TODO(emilk): add "strict" mode where we do pass on the exceptions | ||
| def log_exceptions(func: Callable[..., RT]) -> Callable[..., Optional[RT]]: | ||
| def wrapper_log_exceptions(*args: Any, **kwargs: Any) -> Optional[RT]: |
There was a problem hiding this comment.
Does this interfere with type hinting or are the tools smart enough to trace this through?
There was a problem hiding this comment.
It interferes, I think. The public signature will now have Any as the parameter types.
Apparently things are better in Python 3.10: https://stackoverflow.com/a/68290080
But considering how bad type checking is in Python anyway, I rather have this than leaking exceptions
There was a problem hiding this comment.
I'm also beginning to think that we should consider being much more loose with our typing anyway. We really don't want our types getting in the way of making Rerun easy to use. The main point for us really is documentation
|
This pretty much looks exactly like what I would expect. |
|
With this fix I continue to get type checking functionality in vs-code in python3.10. Would be great to get someone with an older python to confirm this doesn't break anything. |
|
No, it doesn't work for me 😭 :
Same on
|
|
But does it crash? I think typing_extensions makes it valid python… but doesn’t actually include the full functionality. |
It runs fine, but the type hinting is broken which is quite sad |
| RT = TypeVar("RT") # return type | ||
|
|
||
|
|
||
| def log_exceptions(func: Callable[PS, RT]) -> Callable[PS, Optional[RT]]: |
There was a problem hiding this comment.
We can try what @jondo2010 did here: #1186 (comment)
|
I'm gonna give this another go |
|
Replaced by #1477 |
Instead of throwing errors, the logging SDK will now log errors, including call stack. This means the users code will continue to run, just with broken rerun logging.
Checklist
CHANGELOG.md(if this is a big enough change to warrant it)