opendata-stuttgart / feinstaub-map-v2

23 stars 19 forks source link

Fix median #12

Closed PapaBravo closed 5 years ago

PapaBravo commented 5 years ago

The median function of D3 requires the array to be sorted. The documentation is not explicit about it (but mentions it for the similar quantile function).

I have attached a screenshot of the bug manifesting with one outlier.

luftdaten4

As it is hard to force this bug to appear in the life map, I've also attached a few test cases (run with jest) as well.

const { median } = require('../node_modules/d3-array/dist/d3-array');

it('Tests d3 median', () => {
    expect(median([3, 2, 160, 5, 0])).toBe(160);
    expect(median([0, 2, 3, 5, 160])).toBe(3);
    expect(median([3, 2, 160, 5, 0].sort())).toBe(3);
});

it('Tests with complex object', () => {
    const user_selected_value = 'PM10';
    const d_temp = [3, 2, 160, 5, 0].map((v) => ({ o: { data: { PM10: v } } }));

    const result = median(d_temp, (o) => o.o.data[user_selected_value]);
    expect(result).toBe(160);

    const mappedData = d_temp.map(o => o.o.data[user_selected_value]).sort();
    const resultSorted = median(mappedData);
    expect(resultSorted).toBe(3);
});
ricki-z commented 5 years ago

@PapaBravo Thanks for the hint about this. The latest version should contain the appropriate changes. So I will close this pull request without merging.

ricki-z commented 5 years ago

@PapaBravo the version is already published.