Skip to content

TEMPLATE-564 filterInvalidDataPoints exclude Vensim sentinel values#308

Merged
mpqmpqm merged 2 commits intodevelopfrom
TEMPLATE-564
Oct 24, 2025
Merged

TEMPLATE-564 filterInvalidDataPoints exclude Vensim sentinel values#308
mpqmpqm merged 2 commits intodevelopfrom
TEMPLATE-564

Conversation

@mpqmpqm
Copy link
Copy Markdown
Contributor

@mpqmpqm mpqmpqm commented Oct 9, 2025

Closes TEMPLATE-564.

Improves handling for invalid data values (NaN, Infinity, null, undefined and Vensim sentinel values) in line charts.

@mpqmpqm mpqmpqm requested a review from wglasshusain October 9, 2025 21:07
gemini-code-assist[bot]

This comment was marked as outdated.

@forio forio deleted a comment from gemini-code-assist Bot Oct 9, 2025
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces robust handling for invalid data points such as NaN, Infinity, null, undefined, and Vensim sentinel values. A new isValidDataValue utility function and a filterInvalidDataPoints transformation have been added, and the logic for calculating chart extents has been updated to ignore these invalid values. The changes are well-supported by a comprehensive suite of new tests. The code is well-structured, and I have one suggestion to improve immutability and prevent potential side effects in the new data filtering function.

Comment on lines 294 to 298
return Object.assign(s, {
data: s.data.reduce(function (acum, datum) {
if (datum.y != null) {
acum.push(datum);
}

return acum;
}, [])
data: s.data.filter(function (datum) {
return isValidDataValue(datum.y);
})
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This function mutates the series objects in the series array by using Object.assign(s, ...). While series.map creates a new array, the objects within it are references to the original series objects. Modifying them directly can lead to unexpected side effects in other parts of the application that might be using the original data. To ensure immutability, you should create a shallow copy of the series object before adding the filtered data.

Suggested change
return Object.assign(s, {
data: s.data.reduce(function (acum, datum) {
if (datum.y != null) {
acum.push(datum);
}
return acum;
}, [])
data: s.data.filter(function (datum) {
return isValidDataValue(datum.y);
})
});
return Object.assign({}, s, {
data: s.data.filter(function (datum) {
return isValidDataValue(datum.y);
})
});

@gemini-code-assist
Copy link
Copy Markdown

Hello @mpqmpqm! I'm here to help. It looks like you mentioned me, but didn't specify a command. If you'd like me to perform a code review, summarize the pull request, or need help with available commands, please use one of the following:

  • /gemini review to perform a code review.
  • /gemini summary to get a summary of the pull request.
  • /gemini help to see a list of all available commands.

If you have a specific question or comment, please feel free to ask!

@mpqmpqm mpqmpqm merged commit c7e5801 into develop Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant