Skip to content

style: Apply Ruff rules to most notebooks#2690

Merged
matthewfeickert merged 17 commits intomainfrom
feat/extendedRulesPartDeux
Apr 7, 2026
Merged

style: Apply Ruff rules to most notebooks#2690
matthewfeickert merged 17 commits intomainfrom
feat/extendedRulesPartDeux

Conversation

@kratsg
Copy link
Copy Markdown
Contributor

@kratsg kratsg commented Apr 4, 2026

Pull Request Description

See #2689 for remaining outdated notebooks.

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
* Apply Ruff rules ARG001, F401, F811, F821, F841, NPY002, PERF401,
  PTH123, SIM115 for notebooks.
* Amends PR https://github.com/scikit-hep/pyhf/pull/2688.

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.27%. Comparing base (684224d) to head (c5e0dcd).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2690   +/-   ##
=======================================
  Coverage   98.27%   98.27%           
=======================================
  Files          65       65           
  Lines        4289     4289           
  Branches      461      461           
=======================================
  Hits         4215     4215           
  Misses         46       46           
  Partials       28       28           
Flag Coverage Δ
contrib 98.15% <ø> (ø)
doctest 98.27% <ø> (ø)
unittests-3.10 96.45% <ø> (ø)
unittests-3.11 96.45% <ø> (ø)
unittests-3.12 96.45% <ø> (ø)
unittests-3.13 96.45% <ø> (ø)
unittests-3.9 96.52% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@kratsg kratsg marked this pull request as ready for review April 4, 2026 06:29
@kratsg kratsg requested a review from matthewfeickert April 4, 2026 06:32
@kratsg kratsg self-assigned this Apr 4, 2026
@kratsg kratsg added docs Documentation related style Changes that do not affect the meaning of the code labels Apr 4, 2026
Copy link
Copy Markdown
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

@kratsg These changes delete some of the output images that we had in the notebooks. If we had a system that built all of the notebooks and kept the images for the docs that would be okay, but we don't AFAIK.

@kratsg
Copy link
Copy Markdown
Contributor Author

kratsg commented Apr 4, 2026

@kratsg These changes delete some of the output images that we had in the notebooks. If we had a system that built all of the notebooks and kept the images for the docs that would be okay, but we don't AFAIK.

fixed

@kratsg kratsg requested a review from matthewfeickert April 4, 2026 16:43
Copy link
Copy Markdown
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

In some of the notebooks there are changes which result in code getting commented out but not removed. For example, in docs/examples/notebooks/learn/TensorizingInterpolations.ipynb there is now the comment

# allset_all_histo_nom = histogramssets[:, :, 1]

If this can be commented out we can remove it, yes? Is there any reason to keep this?

Also, a curiosity/question, but not a problem: I'm very happy to see the use of np.random.default_rng now, but is there a linting rule in Ruff that sets all seeds to 42?

np.random.default_rng(seed=42)

That seems like a "not great" lint/formatting rule to have, as people can very reasonably want different seeds (that don't have to be tied to Hitchhiker's Guide to the Galaxy).

Also in docs/examples/notebooks/XML_ImportExport.ipynb we have the import pyhf there at the top even though we do everything through ipython magics at the command line to verify that people have things installed to be able to run the notebook. Can we keep that with a some sort of #noqa ignore?

@kratsg
Copy link
Copy Markdown
Contributor Author

kratsg commented Apr 6, 2026

Also in docs/examples/notebooks/XML_ImportExport.ipynb we have the import pyhf there at the top even though we do everything through ipython magics at the command line to verify that people have things installed to be able to run the notebook. Can we keep that with a some sort of #noqa ignore?

I don't understand this comment.

@matthewfeickert
Copy link
Copy Markdown
Member

matthewfeickert commented Apr 6, 2026

I don't understand this comment.

@kratsg sorry. What I mean is the diff of docs/examples/notebooks/XML_ImportExport.ipynb shows this

image

or

$ git diff upstream/main -- docs/examples/notebooks/XML_ImportExport.ipynb
diff --git a/docs/examples/notebooks/XML_ImportExport.ipynb b/docs/examples/notebooks/XML_ImportExport.ipynb
index d80cde21..9b3d942f 100644
--- a/docs/examples/notebooks/XML_ImportExport.ipynb
+++ b/docs/examples/notebooks/XML_ImportExport.ipynb
@@ -13,8 +13,7 @@
    "metadata": {},
    "outputs": [],
    "source": [
-    "# NB: python -m pip install pyhf[xmlio]\n",
-    "import pyhf"
+    "# NB: python -m pip install pyhf[xmlio]"
    ]
   },
   {

As docs/examples/notebooks/XML_ImportExport.ipynb is a notebook that is all about the CLI AP, and so all of it is executed with ipython magics in the notebooks cells, my memory is that we had the import pyhf at the top as a check to make sure that you had pyhf installed before you started executing things at the CLI.

Can/should we keep this?

@github-project-automation github-project-automation bot moved this from In progress to Review in progress in pyhf v0.8.0 Apr 7, 2026
@github-project-automation github-project-automation bot moved this to In progress in pyhf v0.8.0 Apr 7, 2026
@matthewfeickert matthewfeickert changed the title style: fix notebooks more style: Apply Ruff rules to most notebooks Apr 7, 2026
Copy link
Copy Markdown
Member

@matthewfeickert matthewfeickert left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a bunch @kratsg!

@matthewfeickert matthewfeickert merged commit 7ec8a21 into main Apr 7, 2026
22 checks passed
@matthewfeickert matthewfeickert deleted the feat/extendedRulesPartDeux branch April 7, 2026 19:49
@github-project-automation github-project-automation bot moved this from Review in progress to Done in pyhf v0.8.0 Apr 7, 2026
@kratsg kratsg mentioned this pull request Apr 8, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation related style Changes that do not affect the meaning of the code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants