qoega opened a new pull request, #25236: URL: https://github.com/apache/superset/pull/25236
### SUMMARY We(https://github.com/clickHouse/clickHouse) are using Superset for internal analytics and found Partition charts to be very useful. But bugs such as https://github.com/apache/superset/issues/10586 make this powerful feature almost unusable. - Debugging showed that order of nodes with depth more than 2 is not ordered within parent node. - Issue is related to partition limit feature that removes nodes from a tree if number of nodes under the single parent is higher than the limit. It dropped data without leaving sum of values dropped and thus icicle node scale was inconsistent. - Feature partition threshold never worked as 0-1 float parameter was parsed as integer 0 all the time. - Some recent changes removed time column picker that did not allow to use types other than `Not a time series` - Incorrect logic to compute place of deeper nodes did not start to draw child nodes within parent height Proposed solution: - Feature `Partition threshold` was fixed to get correct setting value - Added time column picker back - A separate node with `<other>` name is created to replace all removed nodes within the same parent. This node has cumulative value of all removed nodes. It allows to see significance of pruned partitions and have correct scale for all depth levels. Partition threshold still allows to improve drawing speed significantly for results with many nodes. Ordering of drawn nodes adds `<other>` node after all remaining nodes within same parent ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF 1. Partition Limit feature BEFORE: Partition Limit=2. Removed partitions are just skipped. 4th level nodes are shuffled(not shown within parent/prefix). <img width="1436" alt="Screenshot 2023-09-07 at 21 21 33" src="https://github.com/apache/superset/assets/2159081/3f5b653e-abc8-4d00-8642-92dcb943759d";> AFTER: Partition Limit=2. Smaller nodes replaced to `<other>`. Child nodes are drawn within height. <img width="1428" alt="Screenshot 2023-09-07 at 21 20 06" src="https://github.com/apache/superset/assets/2159081/293cbd96-a5ce-4fc7-acf1-ac2b531e20a4";> 2. Partition threshold BEFORE: - Feature does not work at all. All nodes are drawn as 0.5 is parsed as 0. - Order of 4th column does not respect parents at all which violates the main icicle visualisation logic <img width="1445" alt="Screenshot 2023-09-07 at 21 21 21" src="https://github.com/apache/superset/assets/2159081/c70b30b8-a577-4f38-a7a0-59db968868de";> AFTER: - Nodes with less than 0.5 share within parent are removed and substituted with `<other>` <img width="1436" alt="Screenshot 2023-09-07 at 21 20 32" src="https://github.com/apache/superset/assets/2159081/12ddb4bc-2e0d-49e5-8f81-ed7f3c15e74b";> 3. No partition limit or threshold BEFORE: - Order of 4th level is incorrect and is mixed between different parents. <img width="1445" alt="Screenshot 2023-09-07 at 21 21 10" src="https://github.com/apache/superset/assets/2159081/88da8c5f-0f61-435a-bf9f-0ac9b5ca838d";> TBD ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> Create a Partition chart. Choose COUNT(*) as metric and add several (3+) dimensions. Set Partition Limit to 0 Set Partition Limit to 2 Set Partition Limit to 0 and Partition Threshold to 0.8 Set Partition Limit to 2 and Partition Threshold to 0.8 Set Partition Limit to 0 and Partition Threshold to 0 ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [x] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
