2120 link data to simulations same color#2617
Conversation
|
Should we move this to v13, or wait for v12.2? |
msevestre
left a comment
There was a problem hiding this comment.
I had left many comments to this code on the other PR. Can you please review
@benjaperez1983 @rwmcintosh ?
I would say: yes, V13 or V12.2 whatsoever, but not V12.1 |
Can you let me know please what PR was that? |
|
I assume this one #2602 |
|
@benjaperez1983 Just converted to draft to prevent unintentional merging before 12.1 is released. |
|
@msevestre Could you re-review please ? Thanks! |
src/OSPSuite.Presentation/Presenters/Charts/SimpleChartPresenter.cs
Outdated
Show resolved
Hide resolved
src/OSPSuite.Presentation/Presenters/Charts/UsedColumnsEventArgs.cs
Outdated
Show resolved
Hide resolved
src/OSPSuite.Presentation/Presenters/Charts/DataBrowserPresenter.cs
Outdated
Show resolved
Hide resolved
msevestre
left a comment
There was a problem hiding this comment.
Just a few minor suggestions. Approved
src/OSPSuite.Presentation/Presenters/Charts/DataBrowserPresenter.cs
Outdated
Show resolved
Hide resolved
src/OSPSuite.Presentation/Presenters/Charts/DataBrowserPresenter.cs
Outdated
Show resolved
Hide resolved
src/OSPSuite.Presentation/Presenters/Charts/DataBrowserPresenter.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull Request Overview
This PR implements functionality to maintain consistent curve colors when linking observed data to simulation outputs. When data columns are linked to simulations (e.g., mapping observed data to simulation curves), the added curves now preserve the same color as their corresponding simulation curves based on matching bottom compartments.
Key Changes:
- Extended
UsedColumnsEventArgsto include anIsLinkedDataToSimulationflag to distinguish between regular data additions and simulation-linked data - Added
BottomCompartmentproperty toDataColumnto enable compartment-based color matching - Modified curve color assignment logic to match colors by compartment when linking data to simulations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| UsedColumnsEventArgs.cs | Added IsLinkedDataToSimulation parameter to track when data is being linked to simulations |
| DataColumnDTO.cs | Set BottomCompartment on the underlying DataColumn during DTO initialization |
| DataBrowserPresenter.cs | Propagated isLinkedDataToSimulations flag through the data selection pipeline |
| ChartEditorPresenter.cs | Updated curve creation to pass the linked data flag and reordered color/style update logic |
| CurveChartExtensions.cs | Implemented compartment-based color matching for linked simulation data |
| DataColumn.cs | Added BottomCompartment property to store compartment information |
| ChartEditorPresenterSpecs.cs | Added test coverage for linked data color matching and updated existing test signatures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tests/OSPSuite.Presentation.Tests/Presentation/ChartEditorPresenterSpecs.cs
Show resolved
Hide resolved
…thub.com/Open-Systems-Pharmacology/OSPSuite.Core into 2120-link-data-to-simulations-same-color
Fixes #2120
Description
Link data to simulation, keep the same color for the added curves
Type of change
Please mark relevant options with an
xin the brackets.How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Reviewer checklist
Mark everything that needs to be checked before merging the PR.
This change requires a documentation updateabove is selectedScreenshots (if appropriate):
Questions (if appropriate):