-
-
Notifications
You must be signed in to change notification settings - Fork 14
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
Improved error messages for bindings #9
Conversation
The implementation of the checks looks good. However, the checks are actually not used. I assume that the relevant code has been removed within a merge or similar? |
precice/precice#547 contains the relevant parts. I will manually move them to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried import precice
from my home
directory (note: not python-bindings
) and got the following error:
>>> import precice
pTraceback (most recent call last):
File "<stdin>", line 1, in <module>
File "precice.pyx", line 12, in init precice
ModuleNotFoundError: No module named 'error_handling'
I assume that error_handling.py
has to be added to the module properly, since otherwise it cannot be found. If you import precice
from within python-bindings
this is no problem, because error_handling.py
is on the path.
I tried to run the examples from write_block_vector_data
and ran into some more confusing error messages
Need to provide a good error message for: #74 |
#74 gives an example how we can deal with certain kinds of errors and how we can also test that under certain conditions the correct kind of error is thrown. |
Some more things that we should check (not covered by #74):
Probably there is much more, but this is what currently comes to my mind. |
At the moment only the existing assertion statements have been made more descriptive. There is scope for adding further checks and this is up for discussion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally helpful error messages. If you want, you can also provide the values that go into the assertion. I'm giving an example below. But this is optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the following test fails: https://github.com/precice/python-bindings/pull/9/checks?check_run_id=2176968006
Rest from my side this PR is ready to merge.
I think the failing test should have been fixed on develop already. Just merged via 1d76a09 and hope it's working now. |
Migrated precice/precice#547 here.
Closes #74, closes #7