Skip to content

Commit

Permalink
fix: Allow double and float types to be rollupable
Browse files Browse the repository at this point in the history
- We had this restriction in the UI, but the engine can actually handle it
- Tested with doubles, floats
- Tested with the following tables:
```python
from deephaven import empty_table

doubles = empty_table(1_000_000).update_view(["Group = i*10.4", "N = (i % 347)*0.1", "M = (i % 29)*10.2", "Value=i*2.4", "Weight=i*4.2"])
floats = empty_table(1_000_000).update_view(["Group = i*10.4", "N = (float)(i % 347)*0.1", "M = (float)(i % 29)*10.2", "Value=(float)i*2.4", "Weight=(float)i*4.2"])
```
- Updated e2e tests and unit tests
  • Loading branch information
mofojed committed Dec 12, 2024
1 parent d884bff commit bf7856b
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
21 changes: 21 additions & 0 deletions packages/iris-grid/src/sidebar/RollupRows.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { type dh as DhType } from '@deephaven/jsapi-types';
import { TestUtils } from '@deephaven/test-utils';
import RollupRows from './RollupRows';

it('should allow all column types to be groupable', () => {
function testType(type: string, expected = true) {
const column = TestUtils.createMockProxy<DhType.Column>({ type });
expect(RollupRows.isGroupable(column)).toBe(expected);
}

testType('string');
testType('int');
testType('long');
testType('float');
testType('double');
testType('java.lang.String');
testType('java.lang.Integer');
testType('java.lang.Long');
testType('java.math.BigDecimal', false);
testType('java.math.BigInteger', false);
});
5 changes: 4 additions & 1 deletion packages/iris-grid/src/sidebar/RollupRows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ class RollupRows extends Component<RollupRowsProps, RollupRowsState> {
}

static isGroupable(column: dh.Column): boolean {
return !TableUtils.isDecimalType(column.type);
return (
!TableUtils.isBigDecimalType(column.type) &&
!TableUtils.isBigIntegerType(column.type)
);
}

constructor(props: RollupRowsProps) {
Expand Down
12 changes: 12 additions & 0 deletions tests/table-operations.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,18 @@ test('rollup rows and aggregrate columns', async ({ page }) => {
await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
});

await test.step('Rollup a double column', async () => {
const doubleColumn = page.getByRole('button', {
name: 'Double',
exact: true,
});
expect(doubleColumn).toBeTruthy();
await doubleColumn.dblclick();

await waitForLoadingDone(page);
await expect(page.locator('.iris-grid-column')).toHaveScreenshot();
});

await test.step('Aggregate columns', async () => {
await page.getByText('Constituents').click();
await page.getByText('Non-Aggregated Columns').click();
Expand Down

0 comments on commit bf7856b

Please sign in to comment.