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

Replace nbtest with nbval. #12849

Closed
wants to merge 1 commit into from
Closed

Replace nbtest with nbval. #12849

wants to merge 1 commit into from

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Feb 24, 2023

Description

This PR replaces the nbtest notebook testing shell scripts with a simpler package nbval that integrates with pytest to run notebooks. This allows us to get rid of a fair amount of code and also get more helpful CI outputs.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

rapids-logger "Test script exiting with value: $EXITCODE"
exit ${EXITCODE}
# Run all notebooks with nbval
python -m pytest -v --nbval --nbval-lax notebooks/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to exclude notebooks directory? Because the performance notebook being added in #12595 will require state-of-the-art hardware hence it is not fit to be run in CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll look at how to do exclusions soon. We didn’t run dask notebooks with the old method, so I’ll investigate how to align with that.

For curiosity, what is the approximate runtime of the performance notebook @galipremsagar ?

Copy link
Contributor

@galipremsagar galipremsagar Feb 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hours(~3) on H100, primarily because of CPU parts. GPU parts finish quickly (Max 1-2 mins?).

@vyasr
Copy link
Contributor

vyasr commented Jan 23, 2024

@bdice do you still want to pursue this? The package has been quiet for about a year now, but maybe it's widely used enough that we can still rely on the community keeping it afloat.

@bdice
Copy link
Contributor Author

bdice commented Feb 7, 2024

Nope. I agree that nbval is not as active as nbtest. I thought the pytest integration of nbval could be nice, but it's not worth changing. I'll close this.

@bdice bdice closed this Feb 7, 2024
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.

3 participants