fix(heatmap): y-axis sorts in order - #36302
Conversation
ECharts automatically creates axis categories from data in order of first appearance, ignoring SQL ORDER BY. This causes axes to display in incorrect order when some data combinations are missing. Fix by: - Extracting unique axis values from the dataset - Sorting them according to user's sort configuration (sortXAxis/sortYAxis) - Explicitly providing sorted arrays to ECharts via xAxis.data and yAxis.data Supports all sort options: - alpha_asc/alpha_desc: Lexicographic sorting - value_asc/value_desc: Sort by aggregated metric values Fixes apache#33245
Fixes two critical issues with heatmap axis sorting: 1. Series data now uses axis indices instead of raw values - ECharts requires [xIndex, yIndex, value] format when explicit axis data is provided - Previously used [xValue, yValue, value] causing blank rows 2. Numeric values now sort numerically instead of alphabetically - Detects numeric values and sorts them as numbers (1, 2, 10) - Previously sorted as strings resulting in wrong order (1, 10, 2) - Falls back to locale-aware string sorting for non-numeric values Added tests for: - Numeric sorting validation - Axis index conversion - Mixed numeric/string value handling Related to apache#33245
- Import and use DataRecordValue type for better type safety - Add comprehensive JSDoc comments to helper functions - Add explicit Map<DataRecordValue, number> typing - Remove unnecessary type assertions with proper Map typing - Improve code comments for clarity
- Remove superset.code-workspace (should not be committed) - Add *.code-workspace pattern to .gitignore - Consolidate 16 tests down to 8 focused tests: - Merged 4 alphabetical sorting tests into 1 comprehensive test - Removed redundant 'preserve axis data structure' test - Kept all critical tests for bug fixes (numeric sorting, indices) - All 8 tests passing
Code Review Agent Run #439ea5Actionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
There was a problem hiding this comment.
Pull request overview
This PR fixes the heatmap y-axis sorting issue (#33245) by implementing proper sorting logic for both x and y axes. The implementation introduces two new helper functions to extract unique axis values and sort them based on user-specified options (alphabetical or by metric value), with special handling for numeric values to ensure they sort numerically rather than lexicographically.
Key Changes:
- Added
extractUniqueValues()andsortAxisValues()functions to handle axis sorting with support for alphabetical (numeric-aware) and value-based sorting - Modified series data to use axis indices instead of raw values, enabling proper sorted axis display in ECharts
- Added comprehensive test coverage with 8 test cases covering various sorting scenarios and edge cases
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts | Core implementation of axis sorting logic and index-based data transformation |
| superset-frontend/plugins/plugin-chart-echarts/test/Heatmap/transformProps.test.ts | Comprehensive test suite covering sorting scenarios, null handling, and numeric vs alphabetical sorting |
| .gitignore | Added VS Code workspace files to gitignore |
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
…nsformProps.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
CodeAnt AI is reviewing your PR. |
|
CodeAnt AI finished reviewing your PR. |
Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 3345eb3)
Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> (cherry picked from commit 3345eb3)
Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Evan Rusackas <evan@preset.io> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
SUMMARY
Fixes #33245. Replaces the attempt in #34547.
Note: I leaned heavily on AI coding assistance for this PR since I'm still new to TypeScript. The logic makes sense to me and I verified the desired behaviors in my test environment, but someone familiar with TypeScript should review with care.
I left the addition to .gitignore in, it seems like a developer pattern that might help others.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
BEFORE - 6.0.0rc3
The y-axis is out of order:
AFTER
Y-axis order is correct:
TESTING INSTRUCTIONS
Tests included. I deployed to my test environment and checked ascending and descending for y- and x-axes of numeric and categorical variables.
ADDITIONAL INFORMATION