Sorting control not working on numeric x axis

[yousoph]

How to reproduce the bug

1. Create a new Line Chart using the Vehicle Sales dataset.
2. Set quarter as the X-Axis and count(*) as your metric.
3. Click on CREATE CHART.
4. Change the X-Axis Sort By configuration.

Expected results

Sorting should be updated properly

Actual results

Sort on the chart doesn't change and looks like the below with both ascending and descending sorting

Environment

(please complete the following information):

• browser type and version:
• superset version: master
• python version: python --version
• node.js version: node -v
• any feature flags active:

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

Additional context

CASTing the column to varchar will fix the issue, but it's not clear to the end user why the sorting wouldn't work without having to do this

[yousoph]

cc @villebro and @michael-s-molina since it's related to #26087 and some of the related follow ups

[michael-s-molina]

@villebro I created this ECharts example to try possible configurations to fix this issue. You can see that the order of the data array is respected when the axis is of type category but not when it's value. I was not able to find a configuration to sort the x-axis. I was able to find data transform but that's only applicable to the dataset construct. The problem is that our plugin is based on the series.data construct and changing it to be based on the dataset would require a refactor.

Given the problems that we're facing with numerical x-axis, I'm leaning towards reverting all PRs related to that change and reopen #26034. WDYT?

@sfirke for context.

[villebro]

@yousoph we discussed this at length today with @michael-s-molina. In summary, I feel that outside of RTL, inverting a scalar x-axis doesn't really make sense, and is not something we've ever supported: temporal charts have always been assumed to flow from the left to the right. While we could consider adding a control for reversing x-axis for temporal and numerical x-axes*, sorting them by any other column doesn't really make sense, unless assuming them to be categorical first. So in this sense, the x-axis sorting control was not supposed to be present for numerical x-axes. This is, IMO, a bug.

To address this, without breaking conflicting functionality in the 2.x and 3.x branches, I propose the following:

• If the x-axis is temporal or numerical, we expose a new control underneath the x-axis control called "Force categorical" that defaults to false. By default, numerical charts render with the numerical x-axis, similar to how it is in the 2.x releases. When this control is set to true, the x-axis becomes categorical, which it the behavior in 3.x releases.
• Setting this control to true also exposes the old sorting control, making it possible to specify both the sort column and direction.

With regard to introducing RTL support, I think it's a great idea, but not something we probably can't take on right now, and is something that would likely require a huge overhaul of our UI.

* If we really want to support inverted scalar axes, that's already supported in ECharts. So it's really just a question of adding yet another control for it. However, given how cluttered our control panels already are, and how niche this requirement likely is, I'm not sure this is something we should do before we thoroughly plan for official RTL support.

FYI @sfirke @rusackas

[yousoph]

@villebro the proposed solution makes sense to me! One thing I'd propose adding though - since before the fix in #26034, users were able to sort their numeric x-axis, could we mark any numeric x-axis with sorting applied as categorical and keep the sort by controls that were in place so that existing charts don't seem like they broke after the next fix? I think we'd have to look at both the sort ascending checkbox as well as the sort by dropdown, since the chart config could look something like this:

[villebro]

@yousoph your suggestion makes sense to me, and I've rewritten the fix branch to work with charts that were saved prior to these changes. I expect to be opening the PR today as soon as I've updated tests etc.