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

Cleanup composite analysis (backport #1397) #1447

Open
wants to merge 1 commit into
base: stable/0.6
Choose a base branch
from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Apr 22, 2024

Summary

Thanks to #1342 we can cleanup internals of CompositeCurveAnalysis. Not API break and no feature upgrade with this PR.

Details and comments

Previously the curve data and fit summary data are internally created in CurveAnalysis but immediately discarded. The implementation in CurveAnalysis._run_analysis is manually copied to CompositeCurveAnalysis._run_analysis to access these artifact data to create composite artifact data from them. This makes code fragile since developers needed to manually update both base classes. With this PR, implementation of component analysis is encapsulated.


This is an automatic backport of pull request #1397 done by Mergify.

### Summary

Thanks to #1342 we can cleanup internals of `CompositeCurveAnalysis`.
Not API break and no feature upgrade with this PR.

### Details and comments

Previously the curve data and fit summary data are internally created in
`CurveAnalysis` but immediately discarded. The implementation in
`CurveAnalysis._run_analysis` is manually copied to
`CompositeCurveAnalysis._run_analysis` to access these artifact data to
create composite artifact data from them. This makes code fragile since
developers needed to manually update both base classes. With this PR,
implementation of component analysis is encapsulated.

(cherry picked from commit cb37d42)
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