-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
convert position to v2 for Superset load_examples #5515
convert position to v2 for Superset load_examples #5515
Conversation
superset/data/__init__.py
Outdated
@@ -351,7 +366,7 @@ def load_world_bank_health_n_pop(): | |||
secondary_metric='sum__SP_POP_TOTL', | |||
series="country_name",)), | |||
] | |||
misc_dash_slices.append(slices[-1].slice_name) | |||
# misc_dash_slices.append(slices[-1].slice_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover commented line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
superset/data/__init__.py
Outdated
def update_slice_ids(layout_dict, slices): | ||
charts = list( | ||
filter( | ||
lambda component: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional] the more pythonic way of writing this is by using a list comprehension, something like
charts = [
o for o in layout_dict.values()
if isinstance(component, dict) and component['type'] == 'DASHBOARD_CHART_TYPE'
]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
Minor comments, otherwise LGTM |
db115d7
to
0b5d767
Compare
@@ -33,7 +33,17 @@ | |||
|
|||
DATA_FOLDER = os.path.join(config.get("BASE_DIR"), 'data') | |||
|
|||
misc_dash_slices = [] # slices assembled in a "Misc Chart" dashboard | |||
misc_dash_slices = set() # slices assembled in a "Misc Chart" dashboard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in load_multi_line
, some slices are loaded twice. I have to use Set to store misc_dash's slices name, otherwise same slices are added into list twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we worried that the order wont be deterministic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
misc_dash used charts that are generated by other load functions. I guess the order is not quite important.
@@ -183,6 +180,9 @@ def load_examples_run(load_test_data): | |||
print('Loading [Multi Line]') | |||
data.load_multi_line() | |||
|
|||
print('Loading [Misc Charts] dashboard') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
load_multi_line
also added slice into misc_dashboard. i switch the for these 2 load functions, so that generate misc_dashboards after all slices are ready.
Codecov Report
@@ Coverage Diff @@
## master #5515 +/- ##
==========================================
+ Coverage 63.25% 63.25% +<.01%
==========================================
Files 351 351
Lines 22258 22261 +3
Branches 2470 2470
==========================================
+ Hits 14079 14082 +3
Misses 8164 8164
Partials 15 15
Continue to review full report at Codecov.
|
0b5d767
to
4981fda
Compare
update position_json for dashboards from load_examples.
this PR is based on #5543 and #5463.
@mistercrunch @williaster @michellethomas @john-bodley