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

[Table Viz] Table columns not match with group_by control #5329

Merged

Conversation

graceguo-supercat
Copy link

@graceguo-supercat graceguo-supercat commented Jul 3, 2018

screen_shot_2018-07-02_at_11_07_48_pm

timeseries_limit_metric parameter is supposed to be single value but in the slice record I see timeseries_limit_metric=[]. This caused table viz dropped last column.

@michellethomas @GabeLoins

@codecov-io
Copy link

codecov-io commented Jul 3, 2018

Codecov Report

Merging #5329 into master will increase coverage by 0.03%.
The diff coverage is 28.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5329      +/-   ##
==========================================
+ Coverage   61.34%   61.37%   +0.03%     
==========================================
  Files         373      373              
  Lines       23521    23526       +5     
  Branches     2725     2727       +2     
==========================================
+ Hits        14429    14440      +11     
+ Misses       9079     9073       -6     
  Partials       13       13
Impacted Files Coverage Δ
superset/assets/src/explore/store.js 22.36% <0%> (-0.92%) ⬇️
superset/viz.py 81.35% <100%> (ø) ⬆️
...src/explore/components/controls/MetricsControl.jsx 79.59% <50%> (-0.41%) ⬇️
superset/connectors/sqla/models.py 78.13% <0%> (+0.79%) ⬆️
superset/db_engine_specs.py 53.83% <0%> (+0.93%) ⬆️

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 bd47587...5ded0fb. Read the comment docs.

@michellethomas
Copy link
Contributor

Sorry when we were talking about this yesterday I didn't realize that sortBy shouldn't be a list. I think we should figure out why sortBy is a list in this situation.

@graceguo-supercat graceguo-supercat force-pushed the gg-FixTableVizColumn branch 2 times, most recently from 20004fb to a0f386f Compare July 12, 2018 23:22
superset/viz.py Outdated
@@ -509,7 +509,7 @@ def query_obj(self):
'Choose either fields to [Group By] and [Metrics] or '
'[Columns], not both'))

sort_by = fd.get('timeseries_limit_metric') or []
sort_by = fd.get('timeseries_limit_metric') or None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the or None here, if get fails sort_by will be None

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed. thanks!

@graceguo-supercat graceguo-supercat merged commit 0d10cc5 into apache:master Jul 16, 2018
graceguo-supercat pushed a commit to graceguo-supercat/superset that referenced this pull request Jul 19, 2018
timifasubaa pushed a commit to airbnb/superset-fork that referenced this pull request Jul 25, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 27, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Jul 31, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 3, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 3, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 4, 2018
mistercrunch pushed a commit to lyft/incubator-superset that referenced this pull request Aug 4, 2018
@graceguo-supercat graceguo-supercat deleted the gg-FixTableVizColumn branch October 24, 2018 19:46
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@mistercrunch mistercrunch added 🍒 0.27.0 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 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.27.0 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants