Skip to content
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

Count by class is wrong when input array contains values that are not number / non finite number #41

Open
mthh opened this issue Apr 29, 2024 · 1 comment

Comments

@mthh
Copy link
Member

mthh commented Apr 29, 2024

Consider the following code:

let discr = require("statsbreaks")

let data = [1, 1, 2, 2, 3, 3, 3, 3, 4, 5, 5, 5, 6, 7, 8, 'foo', -Infinity, NaN]
let series = new discr.JenksClassifier(data, 2);
let bks = series.classify(3);
let count = series.countByClass();

I think count should be [8, 5, 2] (as if we used [1, 1, 2, 2, 3, 3, 3, 3, 4, 5, 5, 5, 6, 7, 8] as input array) instead of [9, 5, 2, NaN].

The breaks returned are correct (because the input array is filtered in the inner classification function) but in Classifier classes we store the input array before it is filtered :

this._values = values;

A quick fix is simply to store the filtered input array in the line of code shown below (but we'll be redoing this filtering for nothing in the internal classification function).

A better fix might be to avoid doing this filtering twice (and to avoid creating too many new arrays, since doing array.filter(/* some code */).map(/* some code */) creates two new arrays). However, in most cases this shouldn't make any noticeable difference to performance.

@mthh mthh changed the title Count by class is wrong when input array contains non finite number Count by class is wrong when input array contains values that are non number / non finite number Apr 29, 2024
@mthh mthh changed the title Count by class is wrong when input array contains values that are non number / non finite number Count by class is wrong when input array contains values that are not number / non finite number Apr 29, 2024
@mthh
Copy link
Member Author

mthh commented Apr 30, 2024

Interested by a fix ? Any preference between my two options ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant