Skip to content

chore(analysis): remove legacy ana and analytic directories#327

Open
plexoos wants to merge 1 commit into
mainfrom
remove-ana
Open

chore(analysis): remove legacy ana and analytic directories#327
plexoos wants to merge 1 commit into
mainfrom
remove-ana

Conversation

@plexoos
Copy link
Copy Markdown
Member

@plexoos plexoos commented May 13, 2026

Drop obsolete analysis and geometry helper scripts that are no longer part of the supported
workflow. This trims repository maintenance surface and removes a large amount of unused legacy
tooling.

Drop obsolete analysis and geometry helper scripts that are no longer part of the supported
workflow. This trims repository maintenance surface and removes a large amount of unused legacy
tooling.
Copilot AI review requested due to automatic review settings May 13, 2026 15:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@plexoos plexoos requested a review from ggalgoczi May 13, 2026 15:13
@plexoos
Copy link
Copy Markdown
Member Author

plexoos commented May 13, 2026

@ggalgoczi Please let me know if you need or use any of the removed scripts in this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50848e0e58

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread ana/enum_.py
@@ -1,159 +0,0 @@
#!/usr/bin/env python
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Restore SysRap enum generator scripts

This commit deletes ana/enum_.py (and also ana/OpticksPhoton.py), but sysrap/CMakeLists.txt still defines ALL custom targets that execute ${ANADIR}/enum_.py and ${ANADIR}/OpticksPhoton.py (I checked lines 496, 510, 529, and 546 in that file). In a normal SysRap build from a clean checkout, those commands will fail with missing-file errors, so the build pipeline is left in a broken state unless the CMake targets are updated in the same change.

Useful? React with 👍 / 👎.

Comment thread ana/fold.py
return "\n".join(lines)


class Fold(object):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep Fold module until import sites are migrated

Removing ana/fold.py leaves many remaining Python entry points unable to import (for example sysrap/sevt.py and numerous *_test.py scripts still do from opticks.ana.fold import Fold). Any workflow that runs those scripts now will raise ModuleNotFoundError immediately, so this deletion needs either a compatibility shim or coordinated import migration in the same commit.

Useful? React with 👍 / 👎.

@plexoos plexoos self-assigned this May 13, 2026
@ggalgoczi
Copy link
Copy Markdown
Contributor

@ggalgoczi Please let me know if you need or use any of the removed scripts in this PR

No, I do not use any of these scripts. Thank you for refactoring.

However I see the sysrap/CMakeLists.txt:514–560 has three add_custom_target(… ALL …) rules invoking removed scripts in current file.

Copy link
Copy Markdown
Contributor

@ggalgoczi ggalgoczi left a comment

Choose a reason for hiding this comment

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

No, I do not use any of these scripts. Thank you for refactoring.

However I see the sysrap/CMakeLists.txt:514–560 has three add_custom_target(… ALL …) rules invoking removed scripts in current file.

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