Skip to content

Remove the example notebooks and Texas contingency sample - #132

Merged
frmir merged 4 commits into
mainfrom
remove-notebooks-and-contingency-data
Oct 1, 2026
Merged

frmir merged 4 commits into
mainfrom
remove-notebooks-and-contingency-data

Conversation

@albanpuech

@albanpuech albanpuech commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Remove examples/notebooks/ and examples/data/contingency_texas/. Those files are the tutorials, not part of the library.
  • Point the README at gridfm/gridfm-tutorials, which now hosts the notebooks, the Colab links, and the Texas sample.

Test plan

  • Confirm examples/notebooks and examples/data/contingency_texas are gone on the branch
  • Confirm the README tutorial link opens the tutorials repo
  • Confirm the train example still refers to examples/config and examples/data as a local data path

Made with Cursor

…brary.

They belong with the tutorials. The README now points at gridfm-tutorials.

Signed-off-by: Alban Puech <alban.puech2@ibm.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@albanpuech Thanks for the cleanup — this is nicely scoped and matches the "What Belongs in This Repository" section of CONTRIBUTING (moving tutorial/use-case content out of the core library).

What's needed

  • Just let CI finish — pre-commit, pytests, security, and pip-audit are still pending. One note: if pip-audit reports a lightning advisory (PYSEC-2026-3624), that's a known repo-wide blocker being fixed separately, not anything from this PR. DCO, Bandit, CodeQL, Trivy and detect-secrets are already green.
  • No action otherwise: I checked the tree and nothing left in README.md, examples/config, tests/config, or the source still points at the removed examples/notebooks/ or examples/data/contingency_texas/ paths. The gridfm_graphkit train … --data_path examples/data example remains valid.

Since it's a deletion-only change (plus the 2-line README pointer to gridfm-tutorials), there are no new tests/docstrings/deps/YAMLs to add. Looks ready for a maintainer's look once CI is green.

— 🤖 _automated pre-review; a maintainer will follow up_

Signed-off-by: Alban Puech <alban.puech2@ibm.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@albanpuech Thanks for the follow-up commit — the README now has a tutorial link, which addresses the "point users somewhere" goal.

One thing to double-check on that new link: it targets .../gridfm-graphkit/blob/lfe-tutorial/examples/notebooks/Tutorial_opf_colab.ipynb — i.e. a notebook under examples/notebooks/ on the lfe-tutorial branch of this same repo, which is the very directory this PR removes from main. Two small concerns:

  • It's a bit at odds with the PR summary, which says the notebooks now live in gridfm/gridfm-tutorials. Did you mean to point at the tutorials repo instead of a branch of this one?
  • Linking to a feature branch (lfe-tutorial) is fragile — if that branch is renamed/deleted the Colab link breaks. A tag, main of the tutorials repo, or a stable permalink would be sturdier.

Not a blocker, just worth confirming the target is the one you intend. Everything else from before still looks good: deletion-only otherwise, DCO is green on both commits, and the remaining CI (pre-commit, pytests, security, pip-audit) is just re-running after the push. Same caveat as before — if pip-audit flags the lightning advisory (PYSEC-2026-3624), that's a known repo-wide blocker, not this PR.

— 🤖 _automated pre-review; a maintainer will follow up_

Signed-off-by: Alban Puech <alban.puech2@ibm.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@albanpuech Thanks for the follow-up — the new commit (0fb4772, "Remove the tutorial sentence from the README") resolves the concern I raised: it drops the sentence that linked to the fragile lfe-tutorial branch, so the net README diff vs main is now zero. The PR is cleanly deletion-only again (the 7 notebook/CSV files), which is exactly the kind of tutorial/use-case removal the "What Belongs in This Repository" section of CONTRIBUTING asks for.

One optional thought, not a blocker: with the tutorial sentence gone, the README no longer points users anywhere for the notebooks. If you'd like a pointer, a stable link to gridfm/gridfm-tutorials (its main, or a tag/permalink) would be sturdier than the branch link — but it's entirely fine to leave that to a separate PR.

Nothing else outstanding: DCO is green across all three commits, and the remaining CI (pre-commit, pytests, security, pip-audit, CodeQL) is just re-running after the push. Same standing caveat — if pip-audit flags the lightning advisory (PYSEC-2026-3624), that's a known repo-wide blocker, not this PR. Looks ready for a maintainer's look once CI is green.

— 🤖 _automated pre-review; a maintainer will follow up_

Signed-off-by: Alban Puech <alban.puech2@ibm.com>
@romeokienzler

Copy link
Copy Markdown
Collaborator

@albanpuech Thanks — the new commit (4a668f6, "Point the README at the tutorials repository.") does exactly what we discussed: the README now links to gridfm/gridfm-tutorials itself rather than the fragile lfe-tutorial branch, so users still get a pointer to the notebooks and the link won't break if a branch is renamed. That closes the one open thread from before.

The PR is otherwise unchanged — a clean deletion of the 7 notebook/CSV files plus this one-line README pointer, which matches the "What Belongs in This Repository" scope. DCO is green across all four commits; the remaining CI (pre-commit, pytests, security, pip-audit) is just re-running after the push. Standing caveat unchanged — if pip-audit flags the lightning advisory (PYSEC-2026-3624), that's a known repo-wide blocker, not this PR.

Nothing outstanding from my side; looks ready for a maintainer's look once CI is green.

— 🤖 _automated pre-review; a maintainer will follow up_

@albanpuech
albanpuech requested a review from frmir October 1, 2026 17:35
@frmir
frmir merged commit 8a8defb into main Oct 1, 2026
11 checks passed
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