Skip to content

Fixed condition when numBins is in string format, added minimum and maximum input parameters for calculateHistogram #2

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.

Sign up for GitHub

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

Open
fdpamb wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fixed condition when numBins is in string format, added minimum and maximum input parameters for calculateHistogram #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
067edbc
fixed condition when numBins is string
Jul 11, 2019
a95de39
Add minimum and maximum input parameters
Jul 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ var computeIQR = require('compute-iqr');
function calculateHistogram(arr) {
var numBins = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : 0;
var trimTailPercentage = arguments.length > 2 && arguments[2] !== undefined ? arguments[2] : 0.00;
var minimum = arguments.length > 3 && arguments[3] !== undefined ? arguments[3] : null;
var maximum = arguments.length > 4 && arguments[4] !== undefined ? arguments[4] : null;

var bins = [];

Expand All @@ -34,6 +36,10 @@ function calculateHistogram(arr) {

var min = dataCopy[0];
var max = dataCopy[dataCopy.length - 1];
if (!(minimum === null || maximum === null)) {
min = minimum;
max = maximum;
}

if (numBins === 0) {
var sturges = Math.ceil(Math.log2(dataCopy.length)) + 1;
Expand All @@ -55,7 +61,7 @@ function calculateHistogram(arr) {
dataCopy.forEach(function (item) {
var binIndex = Math.floor((item - min) / binSize);
// for values that lie exactly on last bin we need to subtract one
if (binIndex === numBins) {
if (binIndex === numBins || JSON.stringify(binIndex) === numBins) {
Copy link
Owner

@baus baus Aug 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fdpamb I'm just getting around the updating this module. Thanks for the suggestions. I don't think I will merge this change, as I consider it to be responsibility of the module to handle parameters in multiple types. The user can easily convert this a number prior to calling the function

binIndex--;
}
bins[binIndex][1]++;
Expand Down
12 changes: 8 additions & 4 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const computeIQR = require('compute-iqr');
* @returns Array Two dimensional array. First dimension is the index of the bin, and the second index
* is the count. This allows for direct import into ChartJS without having to change the data shape
*/
function calculateHistogram(arr, numBins = 0, trimTailPercentage = 0.00) {
function calculateHistogram(arr, numBins = 0, trimTailPercentage = 0.00, minimum, maximum) {
const bins = [];

let dataCopy = arr.sort((a, b) => a - b);
Expand All @@ -23,8 +23,12 @@ function calculateHistogram(arr, numBins = 0, trimTailPercentage = 0.00) {
dataCopy = dataCopy.filter(x => x <= rightPercentile && x >= leftPercentile);
}

const min = dataCopy[0];
const max = dataCopy[dataCopy.length - 1];
let min = dataCopy[0];
let max = dataCopy[dataCopy.length - 1];
if (!(minimum === undefined || maximum === undefined || minimum === null || maximum === null)) {
min = minimum;
max = maximum;
}

if(numBins === 0){
const sturges = Math.ceil(Math.log2(dataCopy.length)) + 1;
Expand All @@ -47,7 +51,7 @@ function calculateHistogram(arr, numBins = 0, trimTailPercentage = 0.00) {
dataCopy.forEach(item => {
let binIndex = Math.floor((item - min) / binSize);
// for values that lie exactly on last bin we need to subtract one
if (binIndex === numBins) {
if (binIndex === numBins || JSON.stringify(binIndex) === numBins) {
Copy link
Owner

@baus baus Aug 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above. I don't think it is the module's responsibility to accept parameters of multiple types. The user can easily convert to correct type prior to calling the module

binIndex--;
}
bins[binIndex][1]++;
Expand Down