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

Adding option to visualize negative values in Table view #4570

Merged
merged 7 commits into from
Mar 9, 2018
Merged

Adding option to visualize negative values in Table view #4570

merged 7 commits into from
Mar 9, 2018

Conversation

tanvach
Copy link
Contributor

@tanvach tanvach commented Mar 7, 2018

Resolving #4495

Currently, rows with negative values will not show background bars.

The PR adds a new option "Visualize Negative Values" which will show red tinted bars signifying negative values. This will make it useful for situations where negative values are also important (i.e. A/B testing result).

The direction of the bars for positive and negative values are the same since it'll be less cluttered and we can compare the magnitudes easily.

Passes run_tests.sh and npm run test

@michellethomas @williaster @GabeLoins @mistercrunch

screen shot 2018-03-07 at 2 29 18 pm

screen shot 2018-03-07 at 2 29 43 pm

screen shot 2018-03-07 at 2 29 53 pm

@codecov-io
Copy link

codecov-io commented Mar 7, 2018

Codecov Report

Merging #4570 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4570      +/-   ##
==========================================
- Coverage    71.2%   71.17%   -0.03%     
==========================================
  Files         187      187              
  Lines       14785    14809      +24     
  Branches     1083     1085       +2     
==========================================
+ Hits        10527    10540      +13     
- Misses       4255     4266      +11     
  Partials        3        3
Impacted Files Coverage Δ
...rset/assets/javascripts/explore/stores/visTypes.js 70.58% <ø> (ø) ⬆️
...set/assets/javascripts/explore/stores/controls.jsx 38.16% <ø> (ø) ⬆️
superset/jinja_context.py 73.33% <0%> (-4.93%) ⬇️
superset/assets/javascripts/explore/index.jsx 0% <0%> (ø) ⬆️
superset/config.py 92.12% <0%> (ø) ⬆️
...cripts/explore/components/ExploreViewContainer.jsx 0% <0%> (ø) ⬆️
superset/views/core.py 71.2% <0%> (+0.04%) ⬆️
superset/utils.py 88.12% <0%> (+0.18%) ⬆️
...uperset/assets/javascripts/explore/stores/store.js 53.94% <0%> (+1.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 826d063...452c697. Read the comment docs.

@john-bodley
Copy link
Member

john-bodley commented Mar 8, 2018

@tanvach one suggestion would be to have the cell x-axis represent the [min(0, min-value), max(0, max-value)] range, so if you have values of (-10, -5, 10, 40, 20) it would produce the following bars:

|xx        |      
| x        |
|  xx      |
|  xxxxxxxx|
|  xxxx    |

i.e., both negative and positive values could be shown simultaneously and you wouldn't need to include the check box or change the color for visualizing negative values.

@williaster
Copy link
Contributor

Thanks for adding this!

^I agree with @john-bodley 's point that positive and negative should have different directions

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

@john-bodley @williaster Thanks for the suggestion. I experimented with this idea but turns out the major issue is when all values are negative. The chart will then start from the right, which is confusing since it's what we have right now.

Comparing these two cases for all positives and negatives.

screen shot 2018-03-07 at 8 52 22 pm

screen shot 2018-03-07 at 8 51 39 pm

But when the values are mixed it does look pretty nice

screen shot 2018-03-07 at 8 58 22 pm

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

@john-bodley @williaster Comparison with multiple columns. After playing around for a while I feel they both have pros and cons.

  • Color allows the alignment neater and easier to compare magnitudes
  • Direction makes it easy to see positive and negative values when both exist in a column, but pretty messy for positive only column.

Thoughts?

screen shot 2018-03-07 at 10 10 06 pm

screen shot 2018-03-07 at 10 08 14 pm

@john-bodley
Copy link
Member

@tanvach thanks for exploring the options. Negating the negative values and encoding them with a color reminds be of https://square.github.io/cubism/. As you mentioned both approaches have their pros and cons, but I think I prefer the monotone option. @williaster what are your thoughts?

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

Maybe I'm just used to domain coloring since I think negative number is just a positive number with 180 deg phase angle :)

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

@john-bodley @williaster Since I think the best viz type depends on context, I've simply added options: Right Align and Highlight Negative. By default these are on, meaning it will right align the bar charts and coloring negative numbers (to be backwards compatible with current viz style). By turning Right Align off you will get bars going left for negative and right for positive.

screen shot 2018-03-08 at 8 52 19 am

@mistercrunch
Copy link
Member

+1 you'll need some css3 kung fu to make that happen :)

@williaster
Copy link
Contributor

@tanvach thanks for exploring these options! I agree that exposing the controls to the user is ideal given the tradeoffs. a couple more thoughts:

  • I think the label "Highlight negative" and "Right align" don't quite make it obvious what these do. What do you think about "Align by +/- "Color by +/-" (@john-bodley might have thoughts too)
  • I don't think that backwards compatibility is important here. actually I think it's strange that the bars were right-aligned to start, so I would suggest the behavior be either: align everything left, or align by +/-
  • would it be possible to make the red a little more obvious? it's not super differentiable from grey to me

thoughts?

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

@williaster I've made red channel = 150 and it does look better. Here's aligning to the left. Actually I remember we used to have left aligned bars a few versions before. If we're doing this I'll just make these options default to off.

screen shot 2018-03-08 at 11 30 56 am

For the option labels, took out 'by' and looks better
screen shot 2018-03-08 at 11 32 53 am

@john-bodley
Copy link
Member

john-bodley commented Mar 8, 2018

I agree with @williaster regarding not having to preserve backwards compatibility and that left aligned bars make the most sense. Could we just make negative values always to be colored red? I think it wold be confusing to have both Align +/- and Color +/- unchecked.

@williaster
Copy link
Contributor

thanks @tanvach! @john-bodley and I think this looks good but think coloring should probably be on by default as it's possibly confusing for + and - to appear the same if you don't know it's a setting. will merge after that 🚀

sorry for the back and forth, thanks again for adding this!

@tanvach
Copy link
Contributor Author

tanvach commented Mar 8, 2018

@williaster @john-bodley I've pushed the change to make coloring turned on as default. Thanks for reviewing the PR!

@williaster williaster merged commit b512da8 into apache:master Mar 9, 2018
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Adding option to visualize negative values in Table view

* Adding option for highlighting and right aligning

* Fixed typo

* Fixed case and condition

* Formatting

* Aligning left and default changes

* Changing default
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Adding option to visualize negative values in Table view

* Adding option for highlighting and right aligning

* Fixed typo

* Fixed case and condition

* Formatting

* Aligning left and default changes

* Changing default
@mistercrunch mistercrunch added the 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.24.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants