-
Notifications
You must be signed in to change notification settings - Fork 228
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
fix(examples): better error handling and other fixes #438
Conversation
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
If I do any more work on existing examples this weekend I'll add here, otherwise I'll open separate PRs for new examples. |
Also @Tomas2D the e2e tests are still failing, but in a smaller number now. The following examples fail intermittently when run (even outside pytest):
The first three usually fail on ToolInput validation, but sometimes on the old
|
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
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.
Can you move all try/except blocks within example body into the __main__
block?
It would be easier to read and generally look better in the docs (in the future we might stip the __main__
block entirely in the documentation)
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
|
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.
Not sure whether sys.exit(e.explain())
is a good practise.
This was what was generally suggested on stack overflow, I had doubts at first too, but it works really well in practice |
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Signed-off-by: Alex Bozarth <ajbozart@us.ibm.com>
Which issue(s) does this pull-request address?
Ref #336
Description
Various improvements to existing examples:
options.execution.max_iterations
default tomath.inf
to match TS implementationChecklist