-
Notifications
You must be signed in to change notification settings - Fork 16
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
add sum sumBy and CountIf functionality to ResArr #41
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @DoganCK, thanks for the additions. Could you add some basic tests accompanying this PR?
Sure @kMutagene just added them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! last request i promise :D
Could you add xml documentation above the functions, also documenting the parameters? See
FSharpAux/src/FSharpAux.Core/Dictionary.fs
Lines 9 to 11 in a235fd3
/// <summary>Views the collection as an enumerable sequence of pairs. | |
/// The sequence will be ordered by the keys of the dictionary.</summary> | |
/// <param name="table">The input dictionary.</param> |
for example. I know the other functions in this module completely lack this, but let's lead by example 😆
Sure @kMutagene, that's a good idea! I've added docs for the three functions. By the way this is a really useful library, any plans to move it over to FsLab? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @DoganCK !
Happy that you find the lib useful. I have not thought about moving it to fslab because it lacks a data science specific focus, we can however discuss this further.
Added simple functionality to ResizeArray.
They will help Graphoscope code to be more readable and performant.