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

fix: Override error on filterwarnings to pass notebook tests #1841

Merged
merged 27 commits into from
Apr 6, 2022

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Apr 6, 2022

Description

To get the notebook tests to pass given the many warnings that pop up from the Jupyter infrastructure and papermill, override the ini option for filterwarnings (added in PR #1773) with an empty list to disable error on filterwarnings as the notebook tests are testing for notebooks to run with the latest API, not if Jupyter infrastructure is warning free.

The use of PYTHONWARNINGS='default' should be possible, but is causing errors and is now Issue #1840.

Use scrapbook as it is the new name of nteract-scrapbook and nteract-scrapbook is no longer maintained or developed.
Restrict papermill and scrapbook to compatible releases at the patch level for API stability.

Use the papermill.execute_notebook cwd optional argument to change the execution directory safely to avoid having the state of operations change in the test.

Add concurrency group to avoid wasting run time.

Resolves #1725

Checklist Before Requesting Reviewer

  • Tests are passing
  • "WIP" removed from the title of the pull request
  • Selected an Assignee for the PR to be responsible for the log summary

Before Merging

For the PR Assignees:

  • Summarize commit messages into a comprehensive review of the PR
* Use 'scrapbook' as it is the new name of 'nteract-scrapbook', which is no longer
maintained or developed.
* Restrict 'papermill' and 'scrapbook' to compatible releases at the patch level
for API stability.
* Override error on filterwarnings for the 'notebooks' GHA workflow as it is testing
for the notebooks to run with the latest pyhf API, not if the Jupyter infrastructure
is warning free.
* Use the papermill.execute_notebook 'cwd' optional argument to change the execution
directory safely.
* Use a concurrency group to limit the notebook testing workflow to only one run
at a time.
   - c.f. https://docs.github.com/en/actions/using-jobs/using-concurrency

@matthewfeickert matthewfeickert added tests pytest CI CI systems, GitHub Actions fix A bug fix labels Apr 6, 2022
@matthewfeickert matthewfeickert self-assigned this Apr 6, 2022
@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #1841 (d46bc21) into master (e8cb581) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1841   +/-   ##
=======================================
  Coverage   98.16%   98.16%           
=======================================
  Files          68       68           
  Lines        4315     4315           
  Branches      725      725           
=======================================
  Hits         4236     4236           
  Misses         46       46           
  Partials       33       33           
Flag Coverage Δ
contrib 26.44% <ø> (ø)
doctest 60.78% <ø> (ø)
unittests-3.10 96.03% <ø> (ø)
unittests-3.7 96.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8cb581...d46bc21. Read the comment docs.

@matthewfeickert matthewfeickert changed the title fix: Get notebook tests working with modern papermill fix: Ignore filterwarnings to pass notebook tests and update papermill Apr 6, 2022
@matthewfeickert matthewfeickert changed the title fix: Ignore filterwarnings to pass notebook tests and update papermill fix: Override error on filterwarnings to pass notebook tests Apr 6, 2022
@matthewfeickert matthewfeickert marked this pull request as ready for review April 6, 2022 18:16
@matthewfeickert
Copy link
Member Author

And notebooks pass now 👍

notebooks_pass

@matthewfeickert matthewfeickert merged commit 8c581b5 into master Apr 6, 2022
@matthewfeickert matthewfeickert deleted the chore/change-name-to-scrapbook branch April 6, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI CI systems, GitHub Actions fix A bug fix tests pytest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant