Skip to content

First pass at adding std deviation to measure-object #4969

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.

Sign up for GitHub

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

Closed
dfinke wants to merge 6 commits into from
Closed

First pass at adding std deviation to measure-object #4969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
54a29cd
First pass at adding std deviation to measure-object
dfinke Oct 1, 2017
06dfb2f
Use A*A instead of Math.Pow. Throw if -Average not specified and fixe…
dfinke Oct 2, 2017
eb541c5
Added message to the rex file
dfinke Oct 2, 2017
ca4b539
Set Average true if StdDeviation is set
dfinke Oct 3, 2017
806f3a7
Update message
dfinke Oct 3, 2017
c95b829
Update tests
dfinke Oct 3, 2017
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
70 changes: 68 additions & 2 deletions src/Microsoft.PowerShell.Commands.Utility/commands/utility/Measure-Object.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public sealed class GenericMeasureInfo : MeasureInfo
/// </summary>
public GenericMeasureInfo()
{
Average = Sum = Maximum = Minimum = null;
Average = Sum = Maximum = Minimum = StdDeviation = null;
}

/// <summary>
Expand Down Expand Up @@ -72,6 +72,13 @@ public GenericMeasureInfo()
///
/// </summary>
public double? Minimum { get; set; }

/// <summary>
///
/// The Standard Deviation of property values
///
/// </summary>
public double? StdDeviation { get; set; }
}

/// <summary>
Expand All @@ -91,7 +98,7 @@ public sealed class GenericObjectMeasureInfo : MeasureInfo
/// </summary>
public GenericObjectMeasureInfo()
{
Average = Sum = null;
Average = Sum = StdDeviation = null;
Maximum = Minimum = null;
}

Expand Down Expand Up @@ -129,6 +136,13 @@ public GenericObjectMeasureInfo()
///
/// </summary>
public object Minimum { get; set; }

/// <summary>
///
/// The Standard Deviation of property values
///
/// </summary>
public double? StdDeviation { get; set; }
}


Expand Down Expand Up @@ -227,6 +241,8 @@ private class Statistics

// Generic/Numeric statistics
internal double sum = 0.0;
internal double stdDeviation = 0.0;
internal List<double> stdDeviationNumbers = new List<double>();
internal object max = null;
internal object min = null;

Expand Down Expand Up @@ -265,6 +281,27 @@ public MeasureObjectCommand()

#endregion Common parameters in both sets

/// <summary>
/// Set to true if Standard Deviation is to be returned
/// </summary>
/// <value></value>
[Parameter(ParameterSetName = GenericParameterSet)]
public SwitchParameter StdDeviation
{
get
{
return _measureStdDeviation;
}
set
{
_measureStdDeviation = value;
Copy link
Member

@SteveL-MSFT SteveL-MSFT Oct 2, 2017

Choose a reason for hiding this comment

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

stddeviation requires _measureAverage, so I think we should set _measureAverage = true. If _measureAverage = false, perhaps we should error.

Copy link
Author

@dfinke dfinke Oct 2, 2017

Choose a reason for hiding this comment

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

Makes sense. Where should it throw, in CreateGenericMeasureInfo?

Copy link
Member

@SteveL-MSFT SteveL-MSFT Oct 2, 2017

Choose a reason for hiding this comment

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

Yes, I think that's the right place.

Copy link
Collaborator

@iSazonov iSazonov Oct 3, 2017

Choose a reason for hiding this comment

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

I wonder why we request Average parameter if we can set _measureAverage = true here?

Copy link
Member

@SteveL-MSFT SteveL-MSFT Oct 3, 2017

Choose a reason for hiding this comment

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

Thinking about this, it seems that -Average:$false -StdDeviation should not be a valid combination and perhaps a better way to solve this is to make them different parameter sets. The problem here is that depending on which one is set first, you may get unintended behavior -StdDeviation -Average:$false since the code here overwrites _measureAverage.

Copy link
Collaborator

@iSazonov iSazonov Oct 4, 2017

Choose a reason for hiding this comment

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

We need new flag to process a sum. We can set it in BeginProcessing.

if(value == true)
_measureAverage = true;
}
}

private bool _measureStdDeviation;

/// <summary>
/// Set to true is Sum is to be returned
/// </summary>
Expand Down Expand Up @@ -713,6 +750,8 @@ private void AnalyzeNumber(double numValue, Statistics stat)
{
if (_measureSum || _measureAverage)
stat.sum += numValue;
if (_measureStdDeviation)
stat.stdDeviationNumbers.Add(numValue);
}

/// <summary>
Expand Down Expand Up @@ -793,15 +832,41 @@ private MeasureInfo CreateGenericMeasureInfo(Statistics stat, bool shouldUseGene
{
double? sum = null;
double? average = null;
double? stdDeviation = null;
object max = null;
object min = null;

if (!_nonNumericError)
{
if (_measureSum)
sum = stat.sum;

if (_measureAverage && stat.count > 0)
average = stat.sum / stat.count;

if(_measureStdDeviation && !_measureAverage) {
Copy link
Collaborator

@iSazonov iSazonov Oct 3, 2017

Choose a reason for hiding this comment

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

I think it must be Diagnostic.Assert().

Copy link
Member

@SteveL-MSFT SteveL-MSFT Oct 3, 2017

Choose a reason for hiding this comment

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

With different parameter sets, this code is not needed.

ErrorRecord errorRecord = new ErrorRecord(
PSTraceSource.NewArgumentException("Average"),
"AverageSwitchNotSet",
ErrorCategory.InvalidArgument,
null);

errorRecord.ErrorDetails = new ErrorDetails(this, "MeasureObjectStrings", "AverageSwitchNotSet", "Average");
WriteError(errorRecord);
}

if (_measureStdDeviation && _measureAverage && stat.count > 0)
{
var sumOfDerivation = 0.0;

foreach (double n in stat.stdDeviationNumbers)
{
var m = n - (double)average;
Copy link
Collaborator

@iSazonov iSazonov Oct 3, 2017

Choose a reason for hiding this comment

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

Should we move the cast out of foreach ?

sumOfDerivation += m * m;
}

stdDeviation = Math.Round(Math.Sqrt(sumOfDerivation / (stat.stdDeviationNumbers.Count - 1)), 4);
Copy link
Collaborator

@iSazonov iSazonov Oct 3, 2017

Choose a reason for hiding this comment

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

Please clarify why we round? I believe it must be a double.

Copy link
Author

@dfinke dfinke Oct 5, 2017

Choose a reason for hiding this comment

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

I rounded for no particular reason. I can remove it.

}
}

if (_measureMax)
Expand Down Expand Up @@ -838,6 +903,7 @@ private MeasureInfo CreateGenericMeasureInfo(Statistics stat, bool shouldUseGene
gmi.Count = stat.count;
gmi.Sum = sum;
gmi.Average = average;
gmi.StdDeviation = stdDeviation;
if (null != max)
{
gmi.Maximum = (double)max;
Expand Down
3 changes: 3 additions & 0 deletions src/Microsoft.PowerShell.Commands.Utility/resources/MeasureObjectStrings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,7 @@
<data name="NonNumericInputObject" xml:space="preserve">
<value>Input object "{0}" is not numeric.</value>
</data>
<data name="AverageSwitchNotSet" xml:space="preserve">
<value>StdDeviation was requested and requires the average to be calculated, however '-Average' was set to $false.</value>
</data>
</root>
Loading