fslaborg / Deedle

Easy to use .NET library for data and time series manipulation and for scientific programming
http://fslab.org/Deedle/
BSD 2-Clause "Simplified" License
937 stars 195 forks source link

Optional output of Stats.min and Stats.max on series #419

Closed zyzhu closed 6 years ago

zyzhu commented 6 years ago

Stats.min and Stats.max on Series returns optional values by calling trySeriesExtreme. It returns None when the series is empty.

But this is the only stats function in the whole Stats module that returns optional value. Functions such as Stats.mean returns NaN when series is empty. Applying Stats.min and Stats.max on frame will also output missing value if the column is empty, essentially NaN from user's perspective.

Shall we make Stats.min and Stats.max consistent with other functions to return NaN if series is empty? Then users can avoid confusion such as the following SO question https://stackoverflow.com/questions/51606395/deedle-f-find-the-max-rows-within-an-index-group/51611467#51611467

tpetricek commented 6 years ago

Yes, I would be in favour of a breaking change and just returning float.

The only caveat is that returning NaN won't work if we want to make the function work on series of integers, but I think that even throwing an exception (if we have to) is still better than the current design with options...

zyzhu commented 6 years ago

Forgot about integers. I'm afraid this breaking change requires try catch if anybody has implementation with min/max on integer series already.

Another option is to rename the current min/max with tryMin/tryMax. A new implementation of min and max can output NaN or throw an exception for integers.

tpetricek commented 6 years ago

Renaming to tryMin and tryMax sound like a good approach!

zyzhu commented 6 years ago

Closed as it's addressed in https://github.com/fslaborg/Deedle/pull/422