Skip to content

fix(plotly-express): Decorrelate Exchange from Sym in stocks data - #1409

Open
mofojed wants to merge 3 commits into
deephaven:mainfrom
mofojed:fix-stocks-exchange-seed-correlation
Open

fix(plotly-express): Decorrelate Exchange from Sym in stocks data#1409
mofojed wants to merge 3 commits into
deephaven:mainfrom
mofojed:fix-stocks-exchange-seed-correlation

Conversation

@mofojed

@mofojed mofojed commented Sep 2, 2026

Copy link
Copy Markdown
Member

Sym and Exchange were both seeded with Index, so each drew the same underlying uniform value and Exchange became a pure function of Sym. Only 7 of the 15 Sym/Exchange combinations could ever occur (CAT was never on TPET, DOG was always PETX). Offset the exchange seed so it draws from an independent, still deterministic stream.

Sym and Exchange were both seeded with Index, so each drew the same underlying uniform value and Exchange became a pure function of Sym. Only 7 of the 15 Sym/Exchange combinations could ever occur (CAT was never on TPET, DOG was always PETX). Offset the exchange seed so it draws from an independent, still deterministic stream.
@mofojed mofojed self-assigned this Sep 2, 2026
@mofojed
mofojed requested a balanced review from Copilot September 2, 2026 21:24

Copilot AI left a comment

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.

🟢 Approval recommended

The deterministic seed fix and corresponding test and snapshot updates have no unresolved issues.

Pull request overview

Decorrelates deterministic stock symbol and exchange generation while preserving reproducible output.

Changes:

  • Offsets the exchange RNG seed.
  • Updates affected UI tests and documentation snapshots.
File summaries
File Description
tests/ui_table.spec.ts Updates selection expectations.
plugins/ui/docs/snapshots/d17037c4accf100f79072ead620466e1.json Refreshes query-parameter output.
plugins/ui/docs/snapshots/a0ba791beed7cb302f80b3b85244b528.json Refreshes interactive plot output.
plugins/ui/docs/snapshots/74f478444ea2729b5ebfdf089eaad4ea.json Refreshes table serialization.
plugins/ui/docs/snapshots/228ad0434a83ac64357bc939a56e553a.json Refreshes query-parameter output.
plugins/ui/docs/snapshots/146d658ec29ddc0e62334de0ea0fbb9c.json Refreshes UI table output.
plugins/plotly-express/src/deephaven/plot/express/data/data_generators.py Uses a distinct deterministic exchange seed.
plugins/plotly-express/docs/snapshots/85be7fc6e1425eefd9038059092c01a6.json Refreshes event-example data.
plugins/plotly-express/docs/snapshots/49d3fd3db5d3ff802a3dbdf52c76eb1e.json Refreshes example stock data.
Review details
  • Files reviewed: 18/66 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

plotly-express docs preview (Available for 14 days)

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

ui docs preview (Available for 14 days)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

ui docs preview (Available for 14 days)

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

plotly-express docs preview (Available for 14 days)

@mofojed
mofojed requested a review from dsmmcken September 9, 2026 19:00
@mofojed
mofojed marked this pull request as ready for review September 9, 2026 19:00
Copilot AI review requested due to automatic review settings September 9, 2026 19:00

Copilot AI left a comment

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.

🟢 Approval recommended

The seed separation correctly removes same-row correlation, and affected expectations are updated consistently.

Review details
  • Files reviewed: 18/168 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

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.

3 participants