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

Fixes #2484 bug when exporting pop simulation #2503

Merged
merged 1 commit into from
Jan 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 6 additions & 13 deletions src/PKSim.Core/Chart/ChartData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace PKSim.Core.Chart
{
public class ChartData<TX, TY> : IComparer<PaneData<TX, TY>> where TX : IXValue where TY : IYValue
Copy link
Member Author

Choose a reason for hiding this comment

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

Just formatting in this file

public class ChartData<TX, TY> : IComparer<PaneData<TX, TY>> where TX : IXValue where TY : IYValue
{
private readonly IReadOnlyList<IComparer<object>> _fieldValueComparers;
public AxisData Axis { get; private set; }
Expand All @@ -14,14 +14,10 @@ public class ChartData<TX, TY> : IComparer<PaneData<TX, TY>> where TX : IXValue
private readonly Cache<string, PaneData<TX, TY>> _allPanes;
public IReadOnlyList<string> XFieldNames { get; private set; }

public IEnumerable<KeyValuePair<TX, int>> AllXValues
{
get { return _allXValues.All(); }
}
public IEnumerable<KeyValuePair<TX, int>> AllXValues => _allXValues.All();

public ChartData(AxisData axis, IReadOnlyList<IComparer<object>> fieldValueComparers):this(axis, fieldValueComparers,new List<string>(),null)
public ChartData(AxisData axis, IReadOnlyList<IComparer<object>> fieldValueComparers) : this(axis, fieldValueComparers, new List<string>(), null)
{

}

public ChartData(AxisData axis, IReadOnlyList<IComparer<object>> fieldValueComparers, IReadOnlyList<string> xFieldNames, IComparer<TX> xValueComparer)
Expand All @@ -33,10 +29,7 @@ public ChartData(AxisData axis, IReadOnlyList<IComparer<object>> fieldValueCompa
_allPanes = new Cache<string, PaneData<TX, TY>>(x => x.Id, x => null);
}

public ICache<string,PaneData<TX, TY>> Panes
{
get { return _allPanes; }
}
public ICache<string, PaneData<TX, TY>> Panes => _allPanes;

public int Compare(PaneData<TX, TY> x, PaneData<TX, TY> y)
{
Expand All @@ -48,7 +41,7 @@ public int Compare(PaneData<TX, TY> x, PaneData<TX, TY> y)
public void CreatePaneOrder()
{
//without ToList sortedPanes is empty after Clear
var sortedPanes = _allPanes.OrderBy(x => x, this).ToList();
var sortedPanes = _allPanes.OrderBy(x => x, this).ToList();
_allPanes.Clear();
_allPanes.AddRange(sortedPanes);
}
Expand Down Expand Up @@ -82,7 +75,7 @@ from curve in pane.Curves
from xValue in curve.XValues
select xValue;

public void AddPane(PaneData<TX, TY> pane)
public void AddPane(PaneData<TX, TY> pane)
{
_allPanes.Add(pane);
pane.Chart = this;
Expand Down
14 changes: 8 additions & 6 deletions src/PKSim.Core/Mappers/BoxWhiskerChartDataToDataTableMapper.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
using System.Collections.Generic;
using System.Data;
using PKSim.Core.Chart;
using OSPSuite.Utility.Extensions;
using PKSim.Core.Chart;

namespace PKSim.Core.Mappers
{
Expand Down Expand Up @@ -33,16 +33,18 @@ protected override IEnumerable<DataRow> AddSpecificChartValues(DataRow row, Curv
for (var i = 0; i < curveData.XValues.Count; i++)
{
var newRow = row.Table.NewRow();
var yValue = curveData.YValues[i];
newRow.ItemArray = row.ItemArray;
newRow[_xValue] = curveData.XValues[i].ToString(curveData.XAxis);
newRow[_lowerWhisker] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].LowerWhisker);
newRow[_lowerBox] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].LowerBox);
newRow[_median] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].Median);
Copy link
Member Author

Choose a reason for hiding this comment

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

this was the error everywhere. Using the dimension of the Y axis instead of the YDimensiion defined at the curve level (was not easy to find!)

newRow[_upperBox] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].UpperBox);
newRow[_upperWhisker] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].UpperWhisker);
newRow[_lowerWhisker] = yValueForDataTableFor(curveData, yValue.LowerWhisker);
newRow[_lowerBox] = yValueForDataTableFor(curveData, yValue.LowerBox);
newRow[_median] = yValueForDataTableFor(curveData, yValue.Median);
newRow[_upperBox] = yValueForDataTableFor(curveData, yValue.UpperBox);
newRow[_upperWhisker] = yValueForDataTableFor(curveData, yValue.UpperWhisker);
newRow[_variable] = curveData.YAxis.Caption;
newRows.Add(newRow);
}

return newRows;
}
}
Expand Down
27 changes: 15 additions & 12 deletions src/PKSim.Core/Mappers/ChartDataToDataTableMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
using System.Collections.Generic;
using System.Data;
using System.Linq;
using OSPSuite.Core.Domain;
using OSPSuite.Core.Domain.UnitSystem;
using OSPSuite.Utility;
using OSPSuite.Utility.Data;
using OSPSuite.Utility.Extensions;
using PKSim.Core.Chart;
using OSPSuite.Core.Domain;
using OSPSuite.Core.Extensions;
using PKSim.Core.Extensions;

namespace PKSim.Core.Mappers
{
Expand Down Expand Up @@ -73,6 +74,7 @@ private DataTable getRawTableDataBasedOn(ChartData<TXValue, TYValue> chartData,
AddPanesToTable(dataTable, pane, exportForPivot);
AddObservedDataToTable(dataTable, pane, exportForPivot);
}

dataTable.EndLoadData();

return dataTable;
Expand Down Expand Up @@ -149,21 +151,22 @@ private CurveData<TXValue, TYValue> firstCurveDefinedIn(ChartData<TXValue, TYVal
}

protected abstract void AddSpecificChartColumns(DataTable dataTable, CurveData<TXValue, TYValue> curveData, bool exportForPivot);

protected abstract IEnumerable<DataRow> AddSpecificChartValues(DataRow row, CurveData<TXValue, TYValue> curveData, bool exportForPivot);

protected virtual IReadOnlyList<string> GetDataFields(CurveData<TXValue, TYValue> curveData)
{
return new List<string>();
}
protected virtual IReadOnlyList<string> GetDataFields(CurveData<TXValue, TYValue> curveData) => new List<string>();

protected virtual IReadOnlyList<string> GetRowFields(CurveData<TXValue, TYValue> curveData)
{
return new List<string>();
}
protected virtual IReadOnlyList<string> GetRowFields(CurveData<TXValue, TYValue> curveData) => new List<string>();

//For x values, we can use the dimension of the x axis right away as it is always the same for all curves
protected object xValueForDataTableFor<TX, TY>(CurveData<TX, TY> curveData, double value) where TX : IXValue where TY : IYValue => ValueForDataTableFor(curveData.XAxis, curveData.XAxis.Dimension, value);
Copy link
Member Author

Choose a reason for hiding this comment

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

I created two helper methods so that I don't have to duplicate the logic


//For y values, we need to use the dimension specified for each curve as it may be a merged dimension
protected object yValueForDataTableFor<TX, TY>(CurveData<TX, TY> curveData, double value) where TX : IXValue where TY : IYValue => ValueForDataTableFor(curveData.YAxis, curveData.YDimension, value);

protected object ValueForDataTableFor(IWithDisplayUnit withDisplayUnit, double value)
protected object ValueForDataTableFor(IWithDisplayUnit objectWithTargetUnit, IDimension valueDimension, double value)
{
return double.IsNaN(value) ? DBNull.Value : (object) withDisplayUnit.ConvertToDisplayUnit(value);
return double.IsNaN(value) ? DBNull.Value : (object) objectWithTargetUnit.DisplayValue(value, valueDimension);
}
}
}
20 changes: 12 additions & 8 deletions src/PKSim.Core/Mappers/RangeChartDataToDataTableMapper.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using System.Collections.Generic;
using System.Data;
using OSPSuite.Utility.Extensions;
using PKSim.Assets;
using PKSim.Core.Chart;
using OSPSuite.Utility.Extensions;

namespace PKSim.Core.Mappers
{
Expand Down Expand Up @@ -42,16 +42,20 @@ protected override IEnumerable<DataRow> AddSpecificChartValues(DataRow row, Curv
for (var i = 0; i < curveData.XValues.Count; i++)
{
var newRow = row.Table.NewRow();
var xValue = curveData.XValues[i];
Copy link
Member Author

Choose a reason for hiding this comment

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

sometimes it's good to avoid a bit of code dup by introducing a variable lol...oh my

var yValue = curveData.YValues[i];

newRow.ItemArray = row.ItemArray;
newRow[_xMinimumColumn] = ValueForDataTableFor(curveData.XAxis, curveData.XValues[i].Minimum);
newRow[_xValueColumn] = ValueForDataTableFor(curveData.XAxis, curveData.XValues[i].X);
newRow[_xMaximumColumn] = ValueForDataTableFor(curveData.XAxis, curveData.XValues[i].Maximum);
newRow[_xNumberOfIndividualsColumn] = curveData.XValues[i].NumberOfItems;
newRow[_yLowerPercentileColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].LowerPercentile);
newRow[_yValueColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].Median);
newRow[_yUpperPercentileColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].UpperPercentile);
newRow[_xMinimumColumn] = xValueForDataTableFor(curveData, xValue.Minimum);
newRow[_xValueColumn] = xValueForDataTableFor(curveData, xValue.X);
newRow[_xMaximumColumn] = xValueForDataTableFor(curveData, xValue.Maximum);
newRow[_xNumberOfIndividualsColumn] = xValue.NumberOfItems;
newRow[_yLowerPercentileColumn] = yValueForDataTableFor(curveData, yValue.LowerPercentile);
newRow[_yValueColumn] = yValueForDataTableFor(curveData, yValue.Median);
newRow[_yUpperPercentileColumn] = yValueForDataTableFor(curveData, yValue.UpperPercentile);
newRows.Add(newRow);
}

return newRows;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/PKSim.Core/Mappers/ScatterChartDataToDataTableMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ protected override IEnumerable<DataRow> AddSpecificChartValues(DataRow row, Curv
{
var newRow = row.Table.NewRow();
newRow.ItemArray = row.ItemArray;
newRow[curveData.XAxis.Caption] = ValueForDataTableFor(curveData.XAxis, curveData.XValues[i].X);
newRow[curveData.YAxis.Caption] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].Y);
newRow[curveData.XAxis.Caption] = xValueForDataTableFor(curveData, curveData.XValues[i].X);
newRow[curveData.YAxis.Caption] = yValueForDataTableFor(curveData, curveData.YValues[i].Y);
newRows.Add(newRow);
}
return newRows;
Expand Down
18 changes: 8 additions & 10 deletions src/PKSim.Core/Mappers/TimeProfileChartDataToDataTableMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,27 +36,25 @@ protected override void AddSpecificChartColumns(DataTable dataTable, CurveData<T
dataTable.AddColumn<float>(_yUpperValueColumn);
}

private IEnumerable<DataRow> addSpecificChartValues<TX, TY>(DataRow row, CurveData<TX, TY> curveData, bool exportForPivot)
where TX : TimeProfileXValue
where TY : ITimeProfileYValue
private IEnumerable<DataRow> addSpecificChartValues<TY>(DataRow row, CurveData<TimeProfileXValue, TY> curveData, bool exportForPivot) where TY: ITimeProfileYValue
{
var newRows = new List<DataRow>();

for (var i = 0; i < curveData.XValues.Count; i++)
{
var newRow = row.Table.NewRow();
newRow.ItemArray = row.ItemArray;
var xValue = ValueForDataTableFor(curveData.XAxis, curveData.XValues[i].X);

var xValue = xValueForDataTableFor(curveData, curveData.XValues[i].X);
var yValue = curveData.YValues[i];
if (exportForPivot)
newRow[_xValueColumn] = xValue.ConvertedTo<string>();
else
newRow[_xValueColumn] = xValue;

newRow[_yLowerValueColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].LowerValue);
newRow[_yValueColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].Y);
newRow[_yUpperValueColumn] = ValueForDataTableFor(curveData.YAxis, curveData.YValues[i].UpperValue);
newRows.Add(newRow);
newRow[_yLowerValueColumn] = yValueForDataTableFor(curveData, yValue.LowerValue);
newRow[_yValueColumn] = yValueForDataTableFor(curveData, yValue.Y);
newRow[_yUpperValueColumn] = yValueForDataTableFor(curveData, yValue.UpperValue);
newRows.Add(newRow);
}
return newRows;
}
Expand All @@ -68,7 +66,7 @@ protected override void PerformSpecificTransformationOnPivotedTable(DataTable pi
return;

var column = pivotTable.Columns[_xValueColumn];
var xValueColumnAsString = string.Format("{0}AsString", _xValueColumn);
var xValueColumnAsString = $"{_xValueColumn}AsString";
column.ColumnName = xValueColumnAsString;

//make the column the first column of the pivot table
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ public void Plot(IPopulationDataCollector populationDataCollector, PopulationAna

public void Plot(IPopulationDataCollector populationDataCollector, QuantityPKParameter pkParameter, PopulationAnalysisPKParameterField pkParameterField)
{
plotContinousData(populationDataCollector, pkParameter, pkParameterField, x => x.Plot);
plotContinuousData(populationDataCollector, pkParameter, pkParameterField, x => x.Plot);
}

public void Plot(IPopulationDataCollector populationDataCollector, IParameter parameter, PopulationAnalysisParameterField parameterField)
{
plotContinousData(populationDataCollector, parameter, parameterField, x => x.Plot);
plotContinuousData(populationDataCollector, parameter, parameterField, x => x.Plot);
}

public void Plot(IPopulationDataCollector populationDataCollector, string covariateName)
Expand All @@ -84,7 +84,7 @@ public void ResetPlot()
_populationDistributionPresenter.ResetPlot();
}

private void plotContinousData<TObject>(IPopulationDataCollector populationDataCollector, TObject objectToPlot, INumericValueField numericValueField,
private void plotContinuousData<TObject>(IPopulationDataCollector populationDataCollector, TObject objectToPlot, INumericValueField numericValueField,
Func<IPopulationDistributionPresenter, Action<IPopulationDataCollector, TObject, DistributionSettings, IDimension, Unit>> plotFunc)
where TObject : class, IWithDimension

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,20 @@ protected override void Context()
var pane2 = new PaneData<TimeProfileXValue, TimeProfileYValue>(_yAxis) {Caption = "Female"};
_chartData.AddPane(pane1);
_chartData.AddPane(pane2);
var curve1 = new CurveData<TimeProfileXValue, TimeProfileYValue> {Caption = "Liver"};
var curve1 = new CurveData<TimeProfileXValue, TimeProfileYValue> {Caption = "Liver", YDimension = concDimension };
curve1.Add(new TimeProfileXValue(1), new TimeProfileYValue {Y = 10});
curve1.Add(new TimeProfileXValue(2), new TimeProfileYValue {LowerValue = 20, UpperValue = 30});
pane1.AddCurve(curve1);

var curve2 = new CurveData<TimeProfileXValue, TimeProfileYValue> {Caption = "Kidney"};
var curve2 = new CurveData<TimeProfileXValue, TimeProfileYValue> {Caption = "Kidney", YDimension = concDimension };
curve2.Add(new TimeProfileXValue(3), new TimeProfileYValue {Y = 40});
pane2.AddCurve(curve2);

_observedData = DomainHelperForSpecs.ObservedData();
var displayPathMapper = A.Fake<IQuantityPathToQuantityDisplayPathMapper>();
var dimensionRepository = A.Fake<IDimensionRepository>();
//we need to make sure we return a real dimension as this will be use to convert the values
A.CallTo(() => dimensionRepository.MergedDimensionFor(_observedData.FirstDataColumn())).Returns(concDimension);
var observedDataMapper = new DataRepositoryToObservedCurveDataMapper(displayPathMapper, dimensionRepository);
var observedDataCurves = observedDataMapper.MapFrom(_observedData, new ObservedDataCollection());
observedDataCurves.Each(pane1.AddObservedCurve);
Expand Down