Statistical Functions

[kpkalwa]

Statistical Functions added as a part of library

• Standard Deviation
• Variance
• Standard Error
• Statistical Range
• Mean
• Median
• Mode
• Percentile

[coveralls]

Coverage decreased (-0.05%) to 95.17% when pulling e0bc2c8 on kpkalwa:master into 7054a54 on jashkenas:master.

[jgonggrijp]

Thank you for going through the effort to implement these useful statistical functions and submitting them to Underscore, @kpkalwa!

If you don't mind, I think I will merge your functions under a new category in underscore-contrib instead of in Underscore itself. However, let's continue the review over here for the time being, because underscore-contrib doesn't reflect the modular structure yet.

As a general point of order, please keep this PR open and just push new commits to your master branch. I often see people closing their PR and then reopening a fresh PR; please don't do that.

Please make the following additions and changes in order meet the Underscore quality standard.

• Write tests to ensure that all functions do what they are supposed to do. You can add a new statistics.js module in the test/ directory and imitate the structure from test/collections.js.
• Write accurate, conversational comments that are more specific to the particular function. Right now, all your functions have a single comment that goes like Return the standardDeviation element (or element-based computation). That's not very accurate because the collection never has a "standardDeviation element"; the standard deviation is computed from all elements (or element-based computations). Also please add links to Wikipedia entries about the statistics (you can omit that in mean, statRange and sum, since most programmers probably know what those are).
• Since you are specifically expecting a collection as the first argument in each function, please name the parameter collection rather than obj. (This is being changed for the other functions as well in Functional style #2908).
• Make sure that you follow the whitespace conventions of the other modules:
  • A blank line between the imports and the exported function.
  • Spaces after commas, around binary operators and between the parameter list and the opening brace of a function.
  • Two-space indents.
  • A newline character at the end of each file.

See also the comments below. Again, thank you for these very useful functions in Underscore style!

[kpkalwa]

The changes has been made and pushed to the master branch of the repository can you review and close the pull request @jgonggrijp

[jgonggrijp]

Thank you for the hard work, @kpkalwa. This is going in the right direction!

I can tell that you ran npm run build, because you updated the minified bundles. You're not supposed to update the minified bundles, we usually only do that when cutting a new release. Please run git checkout b713f5a6d75b underscore{,-esm}-min.js{,.map} and then commit in order to undo these changes.

It is fine to commit the non-minified bundles that are automatically generated when you run the tests, though. In fact, since you have now run npm install, this will probably happen automatically because of our commit hooks.

The tests are good but the whitespace is not entirely right yet. Please use two-space indents and add a line break at the end of the file.

The comments are much better now, I think you even overdid them a bit. For this reason, I've suggested some ways to make them shorter again below.

After you've processed this review, the next step will be transferring your changes to Underscore-contrib. I'm thinking that maybe you can just be the first person to contribute code in the new modular structure.

[kpkalwa]

The changes has been made and pushed to the master branch of the repository can you review and close the pull request @jgonggrijp

[jgonggrijp]

Good work @kpkalwa. There are just a few whitespace issues remaining to clear up. As far as I'm concerned, you can start porting your contribution to Underscore-contrib.

Clearing up the remaining whitespace issues:

• Use two-space indents instead of four-space indents in the statistics test module and add a line break at the end of the file.
• Process my comments below.

Porting to Underscore-contrib:

• Fork Underscore-contrib.
• In your local clone of Underscore-contrib, create a new branch, for example statistics (git checkout -b statistics).
• Also in your local clone of Underscore-contrib, run yarn to install the development dependencies.
• Create a new modules directory inside the root of the Underscore-contrib project.
• Copy your statistics modules from underscore/modules to underscore-contrib/modules.
• Copy your additions from underscore/modules/index.js to a new file underscore-contrib/modules/index.collections.statistics.js.
• Copy underscore/test/statistics.js to underscore-contrib/test/collections.statistics.js.
• Commit your changes to the statistics branch, push it to your fork, and submit a pull request to upstream Underscore-contrib (the tests will fail, don't worry about that because we've seem them work correctly over here). We'll continue the process from there!

I know I'm asking a lot, but I can promise that your code will end up in an official relase to NPM, that people will be using it and that you will get the credit for it.

[jgonggrijp]

I just went ahead and did another review because the PR turned up in my notifications. If you'd prefer me to wait until you ask for it, please let me know.

[kpkalwa]

@jgonggrijp I was still working on the changes all the changes has been completed you can review the code.

Migrating to Underscore-contrib will take time for me as the the structure is totally different and not modularized.

[jgonggrijp]

@jgonggrijp I was still working on the changes all the changes has been completed you can review the code.

As far as I can tell, only the whitespace in the test module remains to be fixed.

Migrating to Underscore-contrib will take time for me as the the structure is totally different and not modularized.

Please keep the structure you already have; copy your modules as-is. Please follow my instructions very precisely. I'm planning to restructure Underscore-contrib so that it will have (almost) the same structure as Underscore itself. Your contribution will be the first code that follows the new structure.

[kpkalwa]

@jgonggrijp The changes has been Implemented with the two white spaces

[jgonggrijp]

Continuing the review process in documentcloud/underscore-contrib#243.