You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "gerbermichi (via GitHub)" <gi...@apache.org> on 2024/12/16 14:26:02 UTC
[PR] fix: Filter null child names in treeBuilder utility [superset]
gerbermichi opened a new pull request, #31477:
URL: https://github.com/apache/superset/pull/31477
### SUMMARY
This update modifies the treeBuilder utility to filter out null child names. This ensures that the echart sunburst chart does not display null children as <Null>.
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
Before:
<img width="993" alt="image" src="https://github.com/user-attachments/assets/0242690e-2c00-4cb5-b0e9-dce191082ede" />
After:
<img width="943" alt="image" src="https://github.com/user-attachments/assets/9fdadc10-9a76-4fbb-b8fa-929159f31681" />
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
Create a sunburst diagram with multiple hierarchies and some null values.
### 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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-3051619050
@rusackas Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16163939054). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-2546245116
Code LGTM, but I wonder if there's a use case where people will want to know/see null entries. We could add a "display null values" checkbox, and default it to off/false.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4786579428
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | bfce74d537d167dc6dffbeb6acb9c2923f69453b |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a3b7b6370cfe30008ee39fe |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-31477--superset-docs-preview.netlify.app](https://deploy-preview-31477--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpJCsAgDADAv-RcIRCwyw_6DJeIQqoS7KGU_r3e5jAvdG1XH3AALODV1ZCnOWSnw6Qig9XUW8SEXCQq19kid2nPGWe0jvzqLa0YEhMibsy0J4bvBw) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-31477--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4806680220
https://github.com/apache/superset/pull/41442 up for grabs as a fast-follow
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r3465039435
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:
##########
@@ -92,6 +92,18 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'show_null_values',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Show Null Values'),
+ default: true,
+ renderTrigger: true,
+ description: t('Whether to display null values'),
Review Comment:
```suggestion
description: t('Whether to display entries with null values in the hierarchy'),
```
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4041892354
This PR hasn't been updated in a long time and is blocked by pre-commit CI failures. We'll mark it as draft for now — please fix the pre-commit issues, rebase against `master`, and set it back to 'ready for review' when it's ready.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r3465073114
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:
##########
@@ -251,6 +252,7 @@ export default function transformProps(
columnLabels,
metricLabel,
secondaryMetricLabel,
+ !showNullValues,
Review Comment:
**Suggestion:** `showNullValues` is read without a default and then inverted. For saved/legacy charts where this new control is absent from `formData`, the value is `undefined`, so `!showNullValues` becomes `true` and null nodes are filtered unexpectedly. Default this field to `true` when destructuring to preserve existing behavior. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Legacy sunburst charts unexpectedly hide null hierarchy entries.
- ⚠️ Users see different data after upgrade without changing controls.
- ⚠️ Tooltips and totals exclude rows previously visible as <Null>.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:158-191`,
note that `transformProps()` destructures `showNullValues` from `formData` without a
default: `const { ..., showTotal, showNullValues, sliceId } = formData;` (line 189).
2. In the same file at lines 250-255, `treeBuilder` is called with `!showNullValues` as
the fifth argument: `const treeData = treeBuilder(data, columnLabels, metricLabel,
secondaryMetricLabel, !showNullValues);`, so when `showNullValues` is `undefined`,
`filterNullNames` becomes `true`.
3. In `superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:34-40` and
`62-82`, `treeBuilder()` receives `filterNullNames` and, when it is `true`, filters
children via `const validChildren = filterNullNames ? children.filter(child => child.name
!== null) : children;`, so passing `true` hides null-name nodes.
4. Call `transformProps()` with a `chartProps` object whose `formData` omits
`showNullValues` (mimicking a legacy saved Sunburst chart created before
`show_null_values` existed): `showNullValues` evaluates to `undefined`, `!showNullValues`
becomes `true`, and null hierarchy entries are filtered out even though the new UI control
in `superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:97-104`
defaults `show_null_values` to `true` (meaning "show nulls"), resulting in opposite
behavior for such charts.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=89413eebd6794507a7cc7df5efcb2c5d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=89413eebd6794507a7cc7df5efcb2c5d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts
**Line:** 189:255
**Comment:**
*Api Mismatch: `showNullValues` is read without a default and then inverted. For saved/legacy charts where this new control is absent from `formData`, the value is `undefined`, so `!showNullValues` becomes `true` and null nodes are filtered unexpectedly. Default this field to `true` when destructuring to preserve existing behavior.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=e67468edf7cb3eaeb877c63da3421f7a0789d0128df1121fac0ce2f4ad9f881c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=e67468edf7cb3eaeb877c63da3421f7a0789d0128df1121fac0ce2f4ad9f881c&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:
##########
@@ -74,9 +76,12 @@ export function treeBuilder(
0,
)
: metricValue;
+ const validChildren = filterNullNames
+ ? children.filter(child => child.name !== null)
+ : children;
Review Comment:
**Suggestion:** The null filtering only removes children in the recursive branch, so it does not filter null names when the hierarchy has a single level (or at the root node level with no parent to filter it). This leaves `<Null>` entries visible in valid input shapes. Apply filtering to the current level's result as well, not only to `children`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Sunburst with one hierarchy still shows <Null> nodes.
- ⚠️ Top-level null categories remain despite "hide nulls" enabled.
- ⚠️ Users see inconsistent null handling across hierarchy levels.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In `superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:34-40`,
`treeBuilder()` accepts `filterNullNames?: boolean` and in the multi-level branch (lines
62-88) computes `children = treeBuilder(...)` and then `const validChildren =
filterNullNames ? children.filter(child => child.name !== null) : children;`.
2. In the single-level branch at lines 47-60 (`if (!restGroupby.length)`), the function
pushes leaf items with `name` taken from `curData[key][0][curGroupBy]!` without consulting
`filterNullNames` or applying any filter, so names equal to `null` at this level are never
removed.
3. In the multi-level branch at lines 82-88, the code pushes the parent node as
`result.push({ name, children: validChildren, value: metricValue, secondaryValue, groupBy:
curGroupBy });` while never filtering out the parent itself when `name` is `null`, only
its `children`.
4. From
`superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts:249-255`,
when a user disables nulls via the new control (`showNullValues` false) the call
`treeBuilder(..., !showNullValues)` passes `filterNullNames = true`; with data where the
hierarchy has a single level or top-level `columns` entries containing `null`, the
resulting tree still includes nodes whose `name` is `null`, confirming that
root/single-level `<Null>` entries are not filtered.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a3333897a9fc4ff4b6601ada2fb12135&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a3333897a9fc4ff4b6601ada2fb12135&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts
**Line:** 79:81
**Comment:**
*Incomplete Implementation: The null filtering only removes children in the recursive branch, so it does not filter null names when the hierarchy has a single level (or at the root node level with no parent to filter it). This leaves `<Null>` entries visible in valid input shapes. Apply filtering to the current level's result as well, not only to `children`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=9b254dd48d5e8fedf6c170ea0ef4b8edb26d0f270813f31d67d0681959b6a7cb&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F31477&comment_hash=9b254dd48d5e8fedf6c170ea0ef4b8edb26d0f270813f31d67d0681959b6a7cb&reaction=dislike'>👎</a>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r2271237194
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:
##########
@@ -91,6 +91,18 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'show_null_values',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Show Null Values'),
+ default: false,
Review Comment:
```suggestion
default: true,
```
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4792526924
Almost there @gerbermichi, but codeant raises two fair points: `showNullValues` reads as `undefined` for legacy charts (transformProps.ts:255), and single-level/root null names still slip through (treeBuilder.ts:81). Got time to look into these last touch-ups?
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "gerbermichi (via GitHub)" <gi...@apache.org>.
gerbermichi commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-2547683197
Thank you for your feedback, I've update the PR.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-3051643920
@rusackas Ephemeral environment spinning up at http://54.191.49.5:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r2194367820
##########
superset-frontend/plugins/plugin-chart-echarts/src/utils/treeBuilder.ts:
##########
@@ -74,9 +76,12 @@ export function treeBuilder(
0,
)
: metricValue;
+ const validChildren = filterNullNames
+ ? children.filter(child => child.name !== null)
+ : children;
result.push({
name,
- children,
+ children: validChildren,
Review Comment:
When `filterNullNames` is true and every child is null, the parent node will have an empty `children` array but still be included. You may want to drop parent nodes that end up with no valid children to avoid empty branches in the final tree.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4806634604
Oh jeez... I just merged this, having ignored my prior comment. I'll open up a follow-up PR, and link it here. Would appreciate if you can check it out, as this merged as a _kind of_ breaking change for legacy charts. What I'm about to open should make it non-breaking for pre-existing charts, and still support the intent of this PR.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4700176433
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/31477?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.67%. Comparing base ([`e7ecf54`](https://app.codecov.io/gh/apache/superset/commit/e7ecf5479afafca5edfa5610384e5801ef40f7f8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`aa2fe4d`](https://app.codecov.io/gh/apache/superset/commit/aa2fe4df11992af5b7f001827b0a5f9b944752f3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #31477 +/- ##
==========================================
+ Coverage 63.66% 63.67% +0.01%
==========================================
Files 2660 2660
Lines 144559 144588 +29
Branches 33327 33340 +13
==========================================
+ Hits 92029 92067 +38
+ Misses 50891 50882 -9
Partials 1639 1639
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/31477/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/31477/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.30% <100.00%> (+0.02%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/31477?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4781123230
@gerbermichi this already LGTM'd once you added the toggle. Just needs a rebase on `master` and the pre-commit fixes, then back to ready for review. I'll see if I can help with this to move it forward.
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4806515996
Thank you for this! Merging :D
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4782589356
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #31477:
URL: https://github.com/apache/superset/pull/31477#discussion_r2271238542
##########
superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/controlPanel.tsx:
##########
@@ -91,6 +91,18 @@ const config: ControlPanelConfig = {
},
},
],
+ [
+ {
+ name: 'show_null_values',
+ config: {
+ type: 'CheckboxControl',
+ label: t('Show Null Values'),
+ default: false,
Review Comment:
We should set this to use the default/existing behavior. Not sure if this will break your tests, but I'll commit this :)
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #31477:
URL: https://github.com/apache/superset/pull/31477
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4787896354
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: Filter null child names in treeBuilder utility [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #31477:
URL: https://github.com/apache/superset/pull/31477#issuecomment-4793843333
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org