search

fslaborg / FSharp.Stats

statistical testing, linear algebra, machine learning, fitting and signal processing in F#
https://fslab.org/FSharp.Stats/
Other
210 stars 56 forks source link

Quantiles.OfSorted.* XML comments incorrectly say the functions work with unsorted arrays. #186

Closed nhirschey closed 2 years ago

nhirschey commented 2 years ago

Two observations, if you are happy with my suggestions then I will submit PRs with a fix. I think (1) is important but if you don't like (2) below I won't be upset.

  1. The XML comments for functions in Quantile.OfSorted are incorrect. They say that the functions work with unsorted arrays, but they actually require sorted arrays. I propose that the XML comment is changed to say they require sorted arrays. Example
  2. The functions directly inside Quantile such as Quantile.compute create a copy of the input and sort that copy. These thus seem to be designed as low friction interfaces. What about allowing sequences as input, so that users could pass in sequences or lists with slightly less friction?
bvenn commented 2 years ago

Both suggestions sound good to me. Because in the end all functions apply Array.quickSelectInPlace an array conversion is necessary but keeping the input generic should be the way to go. Thanks @nhirschey!