Skip to content

fix: JS charts should handle shifts, read subscription data by key#6076

Merged
niloc132 merged 3 commits intodeephaven:mainfrom
niloc132:6074-subscription-data
Sep 19, 2024
Merged

fix: JS charts should handle shifts, read subscription data by key#6076
niloc132 merged 3 commits intodeephaven:mainfrom
niloc132:6074-subscription-data

Conversation

@niloc132
Copy link
Copy Markdown
Member

Only viewport subscriptions should read data (and formatting) by
position. Also fixes an error where -1 was not returned when an invalid
position was checked in a RangeSet.

Fixes #6074
Fixes #2435

Only viewport subscriptions should read data (and formatting) by
position. Also fixes an error where -1 was not returned when an invalid
position was checked in a RangeSet.

Fixes deephaven#6074
@niloc132 niloc132 added this to the 0.37.0 milestone Sep 17, 2024
@niloc132 niloc132 self-assigned this Sep 17, 2024
.allMatch(arr -> arr.length == indexes.length);
assert cachedData.values().stream().flatMap(m -> m.values().stream()).allMatch(arr -> arr
.reduce((Object val, Any p1, int p2) -> ((Integer) val) + 1, 0) == indexes.length);
// then adds
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

isn't this where you should apply shifts?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

(discussed on videocall) This implementation doesn't care about shifts - the rowSet provided by the subscription is already correctly shifted, we just need to splice the values into the (flat) array so that downstream charting can ingest those arrays directly.

@niloc132 niloc132 merged commit ff5478d into deephaven:main Sep 19, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ticking charts in JS API are broken JS ChartData type doesn't handle shifts correctly

2 participants