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

GetMinMax #675

Closed
jwbruce93 opened this issue Apr 12, 2018 · 2 comments
Closed

GetMinMax #675

jwbruce93 opened this issue Apr 12, 2018 · 2 comments

Comments

@jwbruce93
Copy link
Contributor

public DicomRange<double> GetMinMax(int padding)

Hi,

I noticed one issue with the 'GetMinMax' function. If by chance you have a dataset where all the pixels are the same value and they are equal to the padding value provided, then it will return double.MaxValue / double.MinValue as the range. I think this can be corrected as follows:


public DicomRange<double> GetMinMax(int padding)
{
     // null or empty return a MinMax of 0/0
      if (Data == null || Data.Length == 0)
                return default(DicomRange<double>);

    // set to the first value to start
      var min = Data[0];
      var max = Data[0];
      var data = Data;
      var length = data.Length;

// loop thru the rest of the pixels, updating the min/max if a new min/max is found
      for (var i = 1; i < length; i++)
       {
           var value = data[i];
           if (value == padding) continue;
                if (value > max)  max = value;
           else if (value < min) min = value;
        }
    return new DicomRange<double>(min, max);
}

Best Regards,
Jamie

@Cregganbane
Copy link
Contributor

Looks like the original code tried to pick the min/max without padding? If data[0] is the padding value this will include that value in the final range. That's quite likely to be the case, for example, in CT where padding values are used to define the area outside a cylindrical acquisition region and data[0] is likely to be padding. When all values are the padding value the original code returning double.Max, double.Min to represent the empty range seems valid. @jwbruce93 can you provide a real world example where this is important?

@gofal
Copy link
Contributor

gofal commented May 11, 2018

@Cregganbane there is a similar discussion ongoing in the associated pull request #685. Would be much appreciated to hear what you think about there.

gofal added a commit that referenced this issue Sep 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants