Calculations: Update First * and Last * reducers to exclude NaNs

[nmarrs]

What is this feature?

Exclude NaN values as part of Last * and First * reducers.

NaN.handling.mov

NaN_-1698445566753.json.txt

Why do we need this feature?

NaN is not typically considered a "valid" value and so excluding it makes sense through these reducers as they are designed to return either the first or last "valid" value.

Who is this feature for?

Users that want to receive the first or last "valid" value in their data.

Which issue(s) does this PR fix?:

Fixes #59862

Special notes for your reviewer:

Please check that:

• It works as expected from a user's perspective.
• If this is a pre-GA feature, it is behind a feature toggle.
• The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

[leeoniya]

probably if (v != null && !Number.isNaN(v))

[nmarrs]

ah good catch - Number.isNaN doesn't infer that value is number and instead is unknown

Currently working on adding a test and the reduceFields function isn't playing nicely and still is returning NaN instead of the expected valid value in the test suite 🙃

[nmarrs]

Stats returns:

{
        sum: 200,
        max: 200,
        min: 200,
        logmin: 200,
        mean: 200,
        last: NaN,
        first: NaN,
        lastNotNull: 200,
        firstNotNull: 200,
        count: 3,
        nonNullCount: 1,
        allIsNull: false,
        allIsZero: false,
        range: 0,
        diff: 0,
        delta: 0,
        step: null,
        diffperc: 0,
        previousDeltaUp: true
}

[nmarrs]

I'm a little scared of modifying the doStandardCalcs function but this seems reasonable 🤷

[nmarrs]

@ryantxu this is a change I'm a little worried about in this PR (modifying doStandardCalcs function) - does this seem reasonable?

[nmarrs]

Not sure if we want special handling in this case (i.e. if all NaN return NaN vs null)

[ryantxu]

Is the undefined part required? I think != null covers both null and undefined.

[nmarrs]

foo != null will check if foo is either null or undefined so we should be good with this simplification 👍

[leeoniya]

we could go one step further and also omit Infinity and -Infinity by switching to Number.isFinite() for the whole condition. 🤔

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/isFinite

[ryantxu]

go one step further

I think that may be one step too far, but I do not feel too strongly

[nmarrs]

we could go one step further and also omit Infinity and -Infinity by switching to Number.isFinite() for the whole condition. 🤔

I don't feel super strongly on this either way - though I am not informed on use cases where seeing +/- inf makes sense. Maybe for now we could move forward with just avoiding NaN and see what user sentiment / feedback is for that initial change? What do you think @leeoniya?