Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to add column groups past some unknown limit #5627

Closed
mofojed opened this issue Jun 17, 2024 · 2 comments · Fixed by #5628
Closed

Unable to add column groups past some unknown limit #5627

mofojed opened this issue Jun 17, 2024 · 2 comments · Fixed by #5628
Assignees
Labels
bug Something isn't working

Comments

@mofojed
Copy link
Member

mofojed commented Jun 17, 2024

Description

Trying to create a large table with a lot of columns and column groups stops working after a certain point.

Steps to reproduce

  1. Run the following snippet:
from deephaven.pandas import to_table
import pandas as pd

columns = [f"col{i}" for i in range(250)]
column_groups = [{"name":f"group_{i}", "children":[f"col{i}" for i in range(i*10, (i+1)*10)]} for i in range(25)]
N_COLUMNS = len(columns)
N_ROWS = 100

zeros = [ [0] * N_COLUMNS for _ in range(N_ROWS)]

table = to_table(pd.DataFrame(zeros, columns=columns))

test1 = table.layout_hints(column_groups = column_groups) # no groups
test2 = table.layout_hints(column_groups = column_groups[:13]) # no groups, same for any value bigger than 13
test3 = table.layout_hints(column_groups = column_groups[:10]) # works 

Expected results

  1. Table test2 should appear with 13 column groups of 10 columns each

Actual results

  1. Table test2 appears, but no column groups are visible.

Additional details and attachments

image

Versions

Engine Version: 0.35.0-SNAPSHOT
Web UI Version: 0.82.0
Java Version: 11.0.23
Barrage Version: 0.6.0
Browser Name: Chrome 125
OS Name: Linux
@deephaven/js-plugin-plotly-express: 0.4.0
@deephaven/js-plugin-ui: 0.15.0
@deephaven/js-plugin-ag-grid: 0.1.0

@mofojed mofojed added bug Something isn't working triage labels Jun 17, 2024
@mattrunyon
Copy link
Contributor

mattrunyon commented Jun 17, 2024

This seems to be an engine/JS API issue. I just checked and layoutHints is null on the JS table

The table has the right attribute print(test1.attributes()) so leaning towards JS API

Number of columns also seems like it's not a direct culprit. I changed to groups of 5 and it only displays 19 groups or 95 columns worth compared to 120 in the example w/ 10.

from deephaven.pandas import to_table
import pandas as pd

columns = [f"col{i}" for i in range(250)]
column_groups = [{"name":f"group_{i}", "children":[f"col{i}" for i in range(i*5, (i+1)*5)]} for i in range(50)]
N_COLUMNS = len(columns)
N_ROWS = 100

zeros = [ [0] * N_COLUMNS for _ in range(N_ROWS)]

table = to_table(pd.DataFrame(zeros, columns=columns))

test1 = table.layout_hints(column_groups = column_groups) # no groups
test2 = table.layout_hints(column_groups = column_groups[:20]) # no groups, same for any value bigger than 20
test3 = table.layout_hints(column_groups = column_groups[:19]) # works

@mofojed mofojed self-assigned this Jun 17, 2024
@mofojed
Copy link
Member Author

mofojed commented Jun 17, 2024

When readTableDefinition is called, the attributes value has no items in the map:
image

There is however an "unsent" attribute for LayoutHints set:
image

Looks like we have a cap for how long a table attribute can be before we don't include it: https://github.com/deephaven/deephaven-core/blob/main/extensions/barrage/src/main/java/io/deephaven/extensions/barrage/util/BarrageUtil.java#L106
image

Unsure why that limit was picked, possible we can just bump it up a bit... though there will still be some limit.

When we wire up layout hints through ui.table, we shouldn't have this same limitation.

@mofojed mofojed transferred this issue from deephaven/web-client-ui Jun 17, 2024
@mofojed mofojed removed the triage label Jun 17, 2024
mofojed added a commit to mofojed/deephaven-core that referenced this issue Jun 17, 2024
- Copied over from DHE days
- No need to have a limit on this anymore
- Tested with the snippet in the ticket
- Fixes deephaven#5627
mofojed added a commit to mofojed/deephaven-core that referenced this issue Jul 10, 2024
- Copied over from DHE days
- No need to have a limit on this anymore
- Tested with the snippet in the ticket
- Fixes deephaven#5627
devinrsmith pushed a commit that referenced this issue Jul 12, 2024
- Cherry picked from `5a96faa3aed4268c79118c43655457f77dc36f2b`
- No need to have a limit on this anymore
- Tested with the snippet in the ticket
- Fixes #5627
- Fixes DH-17392 in DHE
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants