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

feat: deprecate simID and recID accessors #100

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wdconinc
Copy link
Contributor

Briefly, what does this PR introduce?

This deprecates the simID and recID fields of the associations, instead of just simply removing them entirely in #51.

See backwards compatibility note below.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue: deprecate duplicated information)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

This will still break ROOT scripts that directly access the data storage layer (leafs in ROOT tree) in the ROOT files. In podio context that's a private interface, but many in EIC have been treating it as public.

Does this PR change default behavior?

Yes, simID and recID will not be stored.

@wdconinc wdconinc requested a review from a team as a code owner January 20, 2025 17:04
@wdconinc wdconinc requested a review from veprbl January 20, 2025 17:04
@wdconinc wdconinc force-pushed the deprecate-simID-recID branch from 221407e to 8dff4ff Compare January 20, 2025 22:04
@ShujieL
Copy link
Contributor

ShujieL commented Jan 22, 2025

seems that recID and simID are still actively used here https://github.com/eic/EICrecon/blob/32070b472cad3bdc10f957e1ff43273c56118a80/src/algorithms/calorimetry/EnergyPositionClusterMerger.h#L154. Would that be a problem?

@wdconinc
Copy link
Contributor Author

Deprecation is in part a strategy to manifest residual usage.

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.

2 participants