fix(plugin-chart-echarts): support forced categorical x-axis #26404

villebro · 2024-01-03T23:59:08Z

SUMMARY

This PR adds the option to force a chart with a numerical x-axis to categorical. This makes it possible to sort charts that have a numerical x-axis, while still retaining support for numerical x-axis functionality. This should clear up a compatibility issue between previous versions of Superset, where some versions always treated numerical x-axes as categorical, while others always treated them as numerical.

While working on this, I noticed that the Query based datasource was missing the generic data type information that's needed for charts to operate properly. I believe this should resolve some issues we've seen on charts that are using the SQL Lab query as the source.

Note, that I tried to add similar support for temporal x-axes, but that turned out to be very difficult due to how we handle timestamp formats. Therefore, the force option will only be available when the x-axis is numerical, but not for temporal ones. For use cases where similar sorting is needed for temporal charts, I recommend just casting and formatting them to a string based type first.

AFTER

Now there's a new control that makes it possible to force numerical x-axes to categorical (disabled by default):

Note, that if a sorting value had previously been defined, it will set this control to true by default. This is to ensure that charts that had been created on a previous version of Superset, where the x-axis was numeric but were treated as categorical, will work as they were originally intended.

Here we can see that we're able to sort by the metric value if the chart is forced to categorical style:

BEFORE

Previously the sorting control was visible, but didn't do anything if the x-axis was numeric:

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue: fixes Sorting control not working on numeric x axis #26323
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- 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

codecov · 2024-01-04T00:10:24Z

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (3a7d76c) 69.15% compared to head (7a61fdf) 66.98%.
Report is 2 commits behind head on master.

Files	Patch %	Lines
...rt-controls/src/shared-controls/customControls.tsx	7.69%	12 Missing ⚠️
...hart-echarts/src/MixedTimeseries/transformProps.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26404      +/-   ##
==========================================
- Coverage   69.15%   66.98%   -2.18%     
==========================================
  Files        1947     1948       +1     
  Lines       76010    76043      +33     
  Branches     8481     8493      +12     
==========================================
- Hits        52565    50934    -1631     
- Misses      21264    22928    +1664     
  Partials     2181     2181

Flag	Coverage Δ
hive	`?`
javascript	`56.50% <63.88%> (+0.01%)`	⬆️
mysql	`78.06% <100.00%> (-0.02%)`	⬇️
postgres	`78.18% <100.00%> (+<0.01%)`	⬆️
presto	`?`
python	`78.31% <100.00%> (-4.54%)`	⬇️
sqlite	`77.77% <100.00%> (+0.94%)`	⬆️
unit	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

villebro · 2024-01-04T23:53:55Z

/testenv up

github-actions · 2024-01-04T23:55:47Z

@villebro Ephemeral environment spinning up at http://34.220.253.149:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

Vitor-Avila · 2024-01-05T02:50:55Z

This is amazing! thank you!

villebro · 2024-01-05T02:53:57Z

/testenv up

github-actions · 2024-01-05T02:57:59Z

@villebro Ephemeral environment spinning up at http://34.217.31.67:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro · 2024-01-05T04:13:54Z

superset-frontend/plugins/plugin-chart-echarts/src/utils/series.ts

-      xAxisSortSeries === SortSeriesType.Name && typeof sortKey === 'string'
-        ? sortKey.toLowerCase()
+      xAxisSortSeries === SortSeriesType.Name
+        ? typeof sortKey === 'string'
+          ? sortKey.toLowerCase()
+          : sortKey


Without this change, sorting by "category name" wouldn't work if the type is numeric

villebro · 2024-01-05T07:10:49Z

/testenv up

github-actions · 2024-01-05T07:15:03Z

@villebro Ephemeral environment spinning up at http://34.216.59.17:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

michael-s-molina · 2024-01-05T13:03:29Z

Thank you for the PR @villebro and for nice tests!

I noticed that a numerical chart is rendering as categorical when you navigate from the dashboard to Explore.

Screen.Recording.2024-01-05.at.09.56.26.mov

I also noticed it's not possible to drill by when the chart is not categorical. I'm not sure if this is a restriction or if numerical bars could be selected too.

Screen.Recording.2024-01-05.at.09.57.54.mov

michael-s-molina · 2024-01-05T13:06:41Z

@yousoph @Vitor-Avila @sfirke Could you help testing this too?

villebro · 2024-01-05T16:53:56Z

I noticed that a numerical chart is rendering as categorical when you navigate from the dashboard to Explore.

Good catch, there must be something in the initialValue hook. I'll look into that now

sfirke · 2024-01-05T17:15:17Z

I will test this but out sick this afternoon so probably not until Monday.

michael-s-molina · 2024-01-05T18:02:36Z

I will test this but out sick this afternoon so probably not until Monday.

I hope you get well soon!

villebro · 2024-01-05T19:26:53Z

/testenv up

github-actions · 2024-01-05T19:31:04Z

@villebro Ephemeral environment spinning up at http://35.91.107.86:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

villebro · 2024-01-05T19:45:12Z

@michael-s-molina the initial value issue should now be fixed. Please check out my new test dashboard: http://35.91.107.86:8080/superset/dashboard/p/qkWeveYvMXr/

villebro · 2024-01-05T22:23:26Z

I also noticed it's not possible to drill by when the chart is not categorical. I'm not sure if this is a restriction or if numerical bars could be selected too.

@michael-s-molina I believe this is by design - @kgabryje can probably add more context on this. Maybe something we can consider working in the near future?

zhaoyongjie · 2024-01-06T09:34:26Z

@villebro Thanks for tackling the sorting issue. If I didn't forget that the x-axis sorting is using a sorting operator, so the data from backend was sorting ready, I tested this use case in Superset 3.0.2 release, but I can't reproduce it.

BTW, theoretically, if we treated a dimension as a categorical, the dimension should be converted to TEXT type in database, but it might be another topic.

Screen.Recording.2024-01-06.at.10.25.45.mov

michael-s-molina · 2024-01-08T11:44:34Z

@michael-s-molina I believe this is by design - @kgabryje can probably add more context on this. Maybe something we can consider working in the near future?

Got it. Thanks for checking.

This reverts commit c615cb5.

github-actions · 2024-01-08T20:04:51Z

Ephemeral environment shutdown and build artifacts deleted.

Vitor-Avila · 2024-01-08T23:57:49Z

@michael-s-molina thanks for tagging me and apologies for the late response. I just tested creating a chart before this fix and tested the upgrade which successfully fixed the chart!

Thanks @villebro for working on this change! 🎉

(cherry picked from commit 219c4a1)

…26404) (cherry picked from commit 219c4a1)

…26404)

pull-request-size bot added the size/L label Jan 3, 2024

villebro force-pushed the villebro/force-categorical branch from 6a0f529 to ce3ca2a Compare January 4, 2024 05:28

john-bodley requested a review from michael-s-molina January 4, 2024 17:49

villebro force-pushed the villebro/force-categorical branch from 1dab26f to b60d84e Compare January 4, 2024 18:39

michael-s-molina requested review from sfirke and yousoph January 4, 2024 18:44

villebro force-pushed the villebro/force-categorical branch 2 times, most recently from badbcc9 to 9bfaf3f Compare January 4, 2024 21:17

villebro requested a review from kgabryje January 4, 2024 22:16

villebro commented Jan 5, 2024

View reviewed changes

villebro requested a review from zhaoyongjie January 5, 2024 04:28

villebro added 6 commits January 8, 2024 09:14

fix multisort visibility

6c7c847

fix category name sort with numeric value

f6b609a

fix initial value

7b0b3fa

remove check on multidim sort in initialValue

cdc9326

clean up code

c615cb5

simplify code

f20e5de

villebro force-pushed the villebro/force-categorical branch from a9e67a7 to f20e5de Compare January 8, 2024 17:22

Revert "clean up code"

7a61fdf

This reverts commit c615cb5.

villebro merged commit 219c4a1 into apache:master Jan 8, 2024
33 checks passed

villebro deleted the villebro/force-categorical branch January 8, 2024 20:04

michael-s-molina added v3.0 Label added by the release manager to track PRs to be included in the 3.0 branch v3.1 Label added by the release manager to track PRs to be included in the 3.1 branch labels Jan 9, 2024

michael-s-molina pushed a commit that referenced this pull request Jan 9, 2024

fix(plugin-chart-echarts): support forced categorical x-axis (#26404)

52109e5

michael-s-molina pushed a commit that referenced this pull request Jan 9, 2024

fix(plugin-chart-echarts): support forced categorical x-axis (#26404)

136c0e3

(cherry picked from commit 219c4a1)

sfirke mentioned this pull request Jan 15, 2024

Bar chart spilling out past x-axis on both ends #26254

Open

josedev-union pushed a commit to Ortege-xyz/studio that referenced this pull request Jan 22, 2024

fix(plugin-chart-echarts): support forced categorical x-axis (apache#…

a0f346e

…26404) (cherry picked from commit 219c4a1)

mistercrunch added 🍒 3.0.3 🍒 3.0.4 🍒 3.1.0 🍒 3.1.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels labels Mar 8, 2024

sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 2024

fix(plugin-chart-echarts): support forced categorical x-axis (apache#…

90331d4

…26404)

mistercrunch added 🍒 3.1.2 🚢 4.0.0 labels Apr 17, 2024

mistercrunch added the 🍒 3.1.3 label Jul 2, 2024

propellerjet mentioned this pull request Jul 18, 2024

Mixed chart doesn't support forced categorical x-axis #29633

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(plugin-chart-echarts): support forced categorical x-axis #26404

fix(plugin-chart-echarts): support forced categorical x-axis #26404

villebro commented Jan 3, 2024 •

edited

Loading

codecov bot commented Jan 4, 2024 •

edited

Loading

villebro commented Jan 4, 2024

github-actions bot commented Jan 4, 2024

Vitor-Avila commented Jan 5, 2024

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

villebro Jan 5, 2024

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

michael-s-molina commented Jan 5, 2024

michael-s-molina commented Jan 5, 2024 •

edited

Loading

villebro commented Jan 5, 2024

sfirke commented Jan 5, 2024 •

edited

Loading

michael-s-molina commented Jan 5, 2024

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

villebro commented Jan 5, 2024

villebro commented Jan 5, 2024

zhaoyongjie commented Jan 6, 2024 •

edited

Loading

michael-s-molina commented Jan 8, 2024

github-actions bot commented Jan 8, 2024

Vitor-Avila commented Jan 8, 2024 •

edited

Loading

fix(plugin-chart-echarts): support forced categorical x-axis #26404

fix(plugin-chart-echarts): support forced categorical x-axis #26404

Conversation

villebro commented Jan 3, 2024 • edited Loading

SUMMARY

AFTER

BEFORE

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

villebro commented Jan 4, 2024

github-actions bot commented Jan 4, 2024

Vitor-Avila commented Jan 5, 2024

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

villebro Jan 5, 2024

Choose a reason for hiding this comment

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

michael-s-molina commented Jan 5, 2024

michael-s-molina commented Jan 5, 2024 • edited Loading

villebro commented Jan 5, 2024

sfirke commented Jan 5, 2024 • edited Loading

michael-s-molina commented Jan 5, 2024

villebro commented Jan 5, 2024

github-actions bot commented Jan 5, 2024

villebro commented Jan 5, 2024

villebro commented Jan 5, 2024

zhaoyongjie commented Jan 6, 2024 • edited Loading

michael-s-molina commented Jan 8, 2024

github-actions bot commented Jan 8, 2024

Vitor-Avila commented Jan 8, 2024 • edited Loading

villebro commented Jan 3, 2024 •

edited

Loading

codecov bot commented Jan 4, 2024 •

edited

Loading

michael-s-molina commented Jan 5, 2024 •

edited

Loading

sfirke commented Jan 5, 2024 •

edited

Loading

zhaoyongjie commented Jan 6, 2024 •

edited

Loading

Vitor-Avila commented Jan 8, 2024 •

edited

Loading