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.

Sign up for GitHub

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

Jump to bottom

fix: dedup groupby in viz.py while preserving order #10633

Merged
merged 1 commit into from
Aug 21, 2020
Merged

fix: dedup groupby in viz.py while preserving order #10633

merged 1 commit into from
Aug 21, 2020

Conversation

mistercrunch
Copy link
Member

closes #10218

mistercrunch
mistercrunch commented Aug 19, 2020
View reviewed changes
superset/tasks/slack_util.py
@@ -37,6 +35,7 @@ def deliver_slack_msg(
body: str,
file: Optional[Union[str, IOBase, bytes]],
) -> None:
config = current_app.config
Copy link
Member Author

Choose a reason for hiding this comment

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

bycatch, the mypy commit hook was triggering on this...

@rusackas rusackas requested a review from villebro August 19, 2020 05:47
villebro
villebro approved these changes Aug 19, 2020
View reviewed changes
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

LGTM, non-blocking nit

superset/viz.py
@@ -322,7 +322,8 @@ def query_obj(self) -> QueryObjectDict:
gb = self.groupby
metrics = self.all_metrics or []
columns = form_data.get("columns") or []
groupby = list(set(gb + columns))
# merge list and dedup while preserving order
groupby = list(OrderedDict.fromkeys(gb + columns))
Copy link
Member

Choose a reason for hiding this comment

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

Nit, as of 3.7 key order is now officially guaranteed (already implicitly so in 3.6), so no need to use OrderedDict any more:

Suggested change
groupby = list(OrderedDict.fromkeys(gb + columns))
groupby = list(dict.fromkeys(gb + columns))

Reference: https://docs.python.org/3.7/tutorial/datastructures.html#dictionaries

Performing list(d) on a dictionary returns a list of all the keys used in the dictionary, in insertion order

Copy link
Member Author

Choose a reason for hiding this comment

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

oh right, I remember reading about how ordering is guaranteed in py3.

@villebro
Copy link
Member

Merging this for upcoming 0.37.1

@villebro villebro merged commit 82dda47 into apache:master Aug 21, 2020
@villebro villebro deleted the fix_preserve_order branch August 21, 2020 05:02
amitmiran137 pushed a commit to amitmiran137/incubator-superset that referenced this pull request Aug 21, 2020
Merge branch 'master' into dynamic-plugin-import
cf13c0a 
* master: (43 commits)
  feat: Getting fancier with Storybook (apache#10647)
  fix: dedup groupby in viz.py while preserving order (apache#10633)
  feat: bump superset-ui for certified tag (apache#10650)
  feat: setup react page with submenu for datasources listview  (apache#10642)
  feat: add certification to metrics (apache#10630)
  feat(viz-plugins): add date formatting to pivot-table (apache#10637)
  fix: controls scroll issue (apache#10644)
  feat: Allow tests files in  /src (plus Label component tests) (apache#10634)
  fix: remove duplicated params and cache_timeout from list_columns; add viz_type to list_columns (apache#10643)
  chore: splitting button stories into separate stories (apache#10631)
  refactor: remove slice level label_colors from dashboard init load (apache#10603)
  feat: card view bulk select (apache#10607)
  style: Label styling/storybook touchups (apache#10627)
  fix: removing unsupported modal sizes (apache#10625)
  feat(datasource): remove deleted columns and update column type on metadata refresh (apache#10619)
  improve documentation for country maps (apache#10621)
  chore: npm audit fix as of 2020-08-15 (apache#10613)
  feat: dataset REST API for distinct values (apache#10595)
  chore: bump react-redux to 5.1.2, whittling console noise (apache#10602)
  fixing console error about bad html attribute (apache#10604)
  ...

# Conflicts:
#	superset-frontend/src/explore/components/ExploreViewContainer.jsx
#	superset-frontend/src/views/App.tsx
#	superset/config.py
villebro pushed a commit to preset-io/superset that referenced this pull request Aug 29, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (apache#10633)
3a31e10
villebro pushed a commit to preset-io/superset that referenced this pull request Sep 2, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (apache#10633)
f7760b9
villebro pushed a commit that referenced this pull request Sep 5, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (#10633)
a037a47
@dpgaspar dpgaspar added the v0.38 label Sep 9, 2020
dpgaspar pushed a commit to preset-io/superset that referenced this pull request Sep 10, 2020
@mistercrunch @dpgaspar
fix: dedup groupby in viz.py while preserving order (apache#10633)
850f189
villebro pushed a commit to preset-io/superset that referenced this pull request Sep 11, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (apache#10633)
2d71e3a
villebro pushed a commit that referenced this pull request Sep 11, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (#10633)
7d62a72
villebro pushed a commit that referenced this pull request Sep 16, 2020
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (#10633)
25d9fb7
Ofeknielsen pushed a commit to ofekisr/incubator-superset that referenced this pull request Oct 5, 2020
@mistercrunch
fix: dedup groupby in viz.py while preserving order (apache#10633)
d2fd7be
auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
@mistercrunch @auxten
fix: dedup groupby in viz.py while preserving order (apache#10633)
150ca5e
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
@mistercrunch @villebro
fix: dedup groupby in viz.py while preserving order (apache#10633)
3a91152
@mistercrunch mistercrunch added 🍒 0.37.2 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@villebro villebro villebro approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS v0.37 v0.37.2 v0.38 🍒 0.37.2 🚢 0.38.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Preserve Column Order in Table CSV Export
3 participants
@mistercrunch @villebro @dpgaspar