Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sfirke opened this issue Dec 12, 2023 · 15 comments · Fixed by #26269
Open

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

sfirke opened this issue Dec 12, 2023 · 15 comments · Fixed by #26269
Assignees

Comments

@sfirke
Copy link
Contributor

sfirke commented Dec 12, 2023

Superset 3.1.0rc2:

image

How it looks previously in 3.0.0:

image

Here's the chart config in 3.1.0rc2:

image

Again there's a numeric x-axis, maybe broken by #26215 ?

@michael-s-molina
Copy link
Member

It looks like that's how ECharts displays the bar chart with numerical x-axis when min is defined as dataMin.

Screenshot 2023-12-12 at 17 27 26

I searched ECharts docs but I was not able to find a configuration that would fix this. It looks like you need to manually set bounds.

@sfirke
Copy link
Contributor Author

sfirke commented Dec 12, 2023

Thanks for looking into this, I'm sorry to hear there's nothing in ECharts to help with this. This wasn't a problem in 3.0.0 so maybe there's a way forward? It's an unfortunate regression.

Manually set bounds don't provide a satisfactory experience here. If I set 2017 as my x-min I get weird whitespace and x-min = 2018 gets me the same as the bad chart I posted. 2017.5 looks okay but creates a silly "2017.5" label, and anyway if my data values are subject to change I would have to keep changing the bounds.

I think my best way forward for now is to cast the x-axis variable to a categorical (varchar). But I would love to see the 3.0.0 behavior return.

@michael-s-molina
Copy link
Member

@villebro do you think there's something we can do here?

But I would love to see the 3.0.0 behavior return.

@sfirke This would bring back the bug of treating numerical values as categories.

@sfirke
Copy link
Contributor Author

sfirke commented Dec 12, 2023

@michael-s-molina is that bug documented / can you link to it? 🙏 EDIT: I see the regression you referenced, #26034

@michael-s-molina
Copy link
Member

@sfirke
Copy link
Contributor Author

sfirke commented Dec 19, 2023

This is persisting for me in 3.1.0rc3:
image

@sfirke sfirke reopened this Dec 19, 2023
@michael-s-molina
Copy link
Member

@villebro It looks like scale will not be sufficient and we'll need to submit a fix to ECharts to unblock the releases.

@villebro
Copy link
Member

@michael-s-molina @sfirke IIUC, the plan was to add a note to UPDATING informing users that Superset won't automatically convert numerical x-axes to categorical, and that they should be manually cast to categorical to get the previous appearance? I feel blocking the release in anticipation of getting this improvement into ECharts first will be excessive. Therefore, if we really do feel like numerical x-axis support is too lacking in the current state, we should probably just roll back to always using categorical x-axis until the upstream change is in place?

@sfirke
Copy link
Contributor Author

sfirke commented Dec 19, 2023

I had mistakenly thought this issue was fixed so thank you for clarifying. As it's not, can we leave it open?

I agree this should not hold up the release of 3.1.0. If we can't fix this without changes in ECharts then it's a matter of which bug is better to live with, this or #26034. I think we keep this one, because there is the workaround of casting the variable to varchar (which I tested and works). I can cast the variables and replace as users report issues.

Thank you both for working on this. I noticed that compared to rc2, this only affects an unstacked bar chart with a dimension - when I stack it, it looks good in 3.1.0rc3. So there seems to be have been progress from the screenshots posted above by @michael-s-molina and in 26269 by @villebro showing single bars spilling over the sides.

@michael-s-molina
Copy link
Member

I think we keep this one, because there is the workaround of casting the variable to varchar (which I tested and works). I can cast the variables and replace as users report issues.

Makes sense.

As it's not, can we leave it open?

Yes. The context in this discussion will also be useful for folks when they find this problem.

@michael-s-molina
Copy link
Member

For anyone reading this thread, here's the summary about this issue:

There's a bug in ECharts when displaying numerical x-axis in bar charts. We'll work with the ECharts team to provide a solution.

While the solution is not available, you can follow the instructions in UPDATING.md

#26034: Fixes a problem where numeric x-axes were being treated as categorical values. As a consequence of that, the way labels are displayed might change given that ECharts has a different treatment for numerical and categorical values. To revert to the old behavior, users need to manually convert numerical columns to text so that they are treated as categories. Check #26159 for more details.

or use the x-axis truncation controls when applicable.

@sfirke
Copy link
Contributor Author

sfirke commented Jan 15, 2024

New fix method: in 3.0.3+ (where this problem first appears), users don't have to manually convert numerical columns to text - there is now a "force categorical" checkbox that instantly applies the fix to a chart 🎉 Implemented in #26404.

@rusackas
Copy link
Member

This still seems to be the case, and now I'm wondering if we should just detect numeric axes and automatically "force categorical" in these instances.

@rumbin
Copy link
Contributor

rumbin commented May 13, 2024

Wouldn't this bring back #26034?

@rusackas
Copy link
Member

Oh... good question... probably so. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants