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

Optionally avoid custom node errors. #637

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

pfebrer
Copy link
Contributor

@pfebrer pfebrer commented Oct 31, 2023

Custom node errors (e.g. NodeCalcError), when shown directly, obscure the real cause of the error by only showing it in the handled exception's traceback. In this PR I introduce a context key to choose whether the original error or a custom node error should be raised. By default, the original error is raised now.

This should make it easier for users to understand what went wrong when running a node or workflow (plots in the viz module, for example)

Custom node errors, when shown directly, obscure the
real cause of the error by only showing it in the handled
exception's traceback. By default, don't raise the custom error
but the original one.

However, still allow the user to request the custom error to
be raised, because it contains useful information like the node
that errored and which inputs was using.
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Attention: 49 lines in your changes are missing coverage. Please review.

Comparison is base (ff0177a) 87.73% compared to head (1002a5e) 87.71%.
Report is 17 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #637      +/-   ##
==========================================
- Coverage   87.73%   87.71%   -0.03%     
==========================================
  Files         362      362              
  Lines       48318    48354      +36     
==========================================
+ Hits        42394    42415      +21     
- Misses       5924     5939      +15     
Files Coverage Δ
src/sisl/_dispatcher.py 69.26% <100.00%> (ø)
src/sisl/io/orca/tests/test_stdout.py 100.00% <100.00%> (ø)
src/sisl/io/siesta/stdout.py 75.18% <100.00%> (-0.44%) ⬇️
src/sisl/io/vasp/tests/test_stdout.py 100.00% <100.00%> (ø)
src/sisl/lattice.py 93.09% <ø> (ø)
src/sisl/nodes/context.py 88.46% <ø> (ø)
src/sisl/typing/_common.py 100.00% <100.00%> (ø)
src/sisl/io/orca/stdout.py 97.07% <88.23%> (-0.54%) ⬇️
src/sisl/io/vasp/stdout.py 94.38% <71.42%> (-1.54%) ⬇️
src/sisl/nodes/node.py 82.45% <42.85%> (-0.56%) ⬇️
... and 2 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zerothi zerothi merged commit 89dcede into zerothi:main Nov 1, 2023
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants