-
Notifications
You must be signed in to change notification settings - Fork 884
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
Switch to using native traceback
#16851
base: branch-24.10
Are you sure you want to change the base?
Conversation
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.
This is fine with me. We could alternatively set this with the config options:
[pytest]
addopts = --tb=native
Move it to config options. |
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.
Thanks @galipremsagar - Seems fine on the dask side.
@@ -41,6 +41,7 @@ pushd python/dask_cudf/dask_cudf | |||
DASK_DATAFRAME__QUERY_PLANNING=True python -m pytest \ | |||
--junitxml="${RAPIDS_TESTS_DIR}/junit-dask-cudf.xml" \ | |||
--numprocesses=8 \ | |||
--dist=worksteal \ |
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.
Curious how much of a speedup you see with this change?
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.
When we switched cudf wide, we were able to cut down 25% of the execution time: #15207 (comment)
/merge |
Description
This PR switches pytest traceback to
native
instead of prettified pytest traceback that takes longer to finish and spits out the source code of the file where the error happens too which is not needed given the time savings.With pytest traceback:
With
native
traceback:Checklist