Skip to content

chore(shipgen): remove unused Pythia6Generator#1275

Merged
olantwin merged 1 commit into
masterfrom
chore/remove-unused-pythia6generator
Jun 22, 2026
Merged

chore(shipgen): remove unused Pythia6Generator#1275
olantwin merged 1 commit into
masterfrom
chore/remove-unused-pythia6generator

Conversation

@olantwin

@olantwin olantwin commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Audit (follow-up to tpythia6Generator error in makeCascade.py #1272 / fix(shipgen): make tPythia6Generator concrete again (#1272) #1274) found Pythia6Generator (no t) has zero callsites: no Python script constructs it, no C++ caller exists, no docs / README / CHANGELOG reference it, no CI workflow exercises it. Its .cxx and .h have only seen cosmetic refactors since the 2008 import.
  • It also has two latent bugs that fix(shipgen): make tPythia6Generator concrete again (#1272) #1274 was about to paper over: it inherits abstract Init overloads from SHiP::Generator without overriding them, and its default constructor leaves raw FILE* / Char_t* pointers indeterminate, crashing the destructor on default-construction.
  • Delete the class rather than fix it. tPythia6Generator is unaffected (it is actively used by macro/makeCascade.py and run_simScript.py --Pythia6). The ROOTEGPythia6 dependency stays for tPythia6Generator.

Test plan

Independent of #1274 — neither PR blocks the other.

Summary by CodeRabbit

  • Removed
    • Removed the legacy text-format event reader. An alternative implementation remains available for users who require this functionality.

Pythia6Generator was a parser for a custom Pythia6 text event-record
format added by S. Spataro in 2008. It has zero callsites anywhere in
the repository (no Python script, no C++ caller, no doc, no CI
workflow). Recent history shows only cosmetic touches (clang-tidy,
NULL→nullptr, copyright, formatting) — no substantive change since
the original import.

It is also a latent bug: like tPythia6Generator before #1272, it
inherits abstract Init overloads from SHiP::Generator without
overriding them, and its default constructor leaves raw FILE* / file
name pointers indeterminate so the destructor crashes if anything ever
default-constructs it.

Remove the class rather than fix it. tPythia6Generator (used by
macro/makeCascade.py and the --Pythia6 branch of run_simScript.py) is
unaffected; the ROOTEGPythia6 dependency stays for its sake.
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR removes the unused legacy Pythia6Generator class by deleting its header (Pythia6Generator.h, 131 lines) and implementation (Pythia6Generator.cxx, 117 lines), dropping Pythia6Generator.cxx from the CMake ShipGen library sources, updating the ROOT LinkDef.h dictionary pragmas to replace Pythia6Generator with tPythia6Generator and add SHiP namespace directives, and recording the removal in CHANGELOG.md.

Changes

Remove legacy Pythia6Generator

Layer / File(s) Summary
Build system and ROOT dictionary cleanup
shipgen/CMakeLists.txt, shipgen/LinkDef.h, CHANGELOG.md
Pythia6Generator.cxx is dropped from the ShipGen CMake SOURCES list. In LinkDef.h, the #pragma link C++ class Pythia6Generator is replaced with #pragma link C++ class tPythia6Generator and new directives for the SHiP namespace and SHiP::Generator. The changelog records the removal under ### Removed.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • ShipSoft/FairShip#1225: Modifies Pythia6Generator::ReadEvent in shipgen/Pythia6Generator.cxx (the same implementation file entirely deleted by this PR).
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: removing the unused Pythia6Generator class from the shipgen module.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed Pull request description provides comprehensive context including detailed summary of rationale, technical justification, test plan verification, and impact clarification.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/remove-unused-pythia6generator

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown
Contributor

CI Plots

View all plots

fixed-target

pixi

PlaneHA20011 PlaneHA21011 PlaneHA22011 PlaneHA10011 PlaneHA11011 PlaneHA12011 PlaneHA20012 PlaneHA21012 PlaneHA22012 PlaneHA10012 PlaneHA11012 PlaneHA12012 PlaneHA20013 PlaneHA21013 PlaneHA22013 PlaneHA10013 PlaneHA11013 PlaneHA12013 PlaneHA20014 PlaneHA21014 PlaneHA22014 PlaneHA10014 PlaneHA11014 PlaneHA12014 PlaneHA20015 PlaneHA21015 PlaneHA22015 PlaneHA10015 PlaneHA11015 PlaneHA12015 PlaneHA20016 PlaneHA21016 PlaneHA22016 PlaneHA10016 PlaneHA11016 PlaneHA12016 PlaneHA20017 PlaneHA21017 PlaneHA22017 PlaneHA10017 PlaneHA11017 PlaneHA12017 PlaneHA10022 PlaneHA11022 PlaneHA12022 PlaneHA10111 PlaneHA11111 PlaneHA12111 PlaneHA10221 PlaneHA11221 PlaneHA12221 PlaneHA10223 PlaneHA11223 PlaneHA12223 PlaneHA10331 PlaneHA11331 PlaneHA12331

sim-chain

helium-all-TRY_2025

distu distv disty nmeas chi2 delPOverP pullPOverPx pullPOverPy pullPOverPz delPOverP2 delPOverPz delPOverP2z chi2 prob IP Vzresol Vxresol Vyresol Vzpull Vxpull Vypull Doca IP0 HNL IP0_pi0 HNL_pi0 HNLw meas meas2 measVSchi2 distu distv disty meanhits extrapTimeDetX extrapTimeDetY oa nrtracks nrSBT pi0Mass delPOverPz_py delPOverP2z_py pullPOverPx_py pullPOverPy_py pullPOverPz_py

vacuums-all-TRY_2025

distu distv disty nmeas chi2 delPOverP pullPOverPx pullPOverPy pullPOverPz delPOverP2 delPOverPz delPOverP2z chi2 prob IP Vzresol Vxresol Vyresol Vzpull Vxpull Vypull Doca IP0 HNL IP0_pi0 HNL_pi0 HNLw meas meas2 measVSchi2 distu distv disty meanhits extrapTimeDetX extrapTimeDetY oa nrtracks nrSBT pi0Mass delPOverPz_py delPOverP2z_py pullPOverPx_py pullPOverPy_py pullPOverPz_py

tracking-benchmark

pixi

h_dp_over_p h_dp_vs_p h_dx h_dy h_dtx h_dty h_hit_residual h_hit_pull h_chi2ndf h_p_truth h_p_matched

@olantwin

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@olantwin olantwin merged commit 47e1f95 into master Jun 22, 2026
15 checks passed
@olantwin olantwin deleted the chore/remove-unused-pythia6generator branch June 22, 2026 14:41
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.

1 participant