Remove the use of Pandas' iloc() in WorldMapViz

[elukey]

This commit changed the value of df[metric]
when the same metric is used in a World Map panel for both bubble size
and an axis (either X or Y). This change removes the use of .iloc that is not needed anymore (and causes exceptions while rendering world maps).

Should fix #7006

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

WorldMap charts, using the the same metric for X/Y axis and bubble size, are not working anymore from 0.29+ due to commit. If I add the following snipped to the WorldMapViz in superset/viz.py everything works as expected again:

        qry['metrics'] = [
            self.form_data['metric'], self.form_data['secondary_metric']]

The .iloc functionality in WorldMap is not needed anymore.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

• Has associated issue:
• Changes UI
• Requires DB Migration.
• Confirm DB Migration upgrade and downgrade tested.
• Introduces new feature or API
• Removes existing feature or API

REVIEWERS

[codecov-io]

Codecov Report

Merging #7379 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #7379   +/-   ##
=======================================
  Coverage   65.26%   65.26%           
=======================================
  Files         430      430           
  Lines       21077    21077           
  Branches     2338     2338           
=======================================
  Hits        13756    13756           
  Misses       7205     7205           
  Partials      116      116

Impacted Files	Coverage Δ
superset/viz.py	71.56% <0%> (ø)	⬆️

---

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 598526a...cdf93f9. Read the comment docs.

[elukey]

@mistercrunch I think I finally found a good explanation of what broke the world map visualization. The change in my opinion is to remove the use of .iloc, from my tests it is the simplest solution and it solves all my visualization issues.

[elukey]

@mistercrunch if you have time for a review, I'd be really happy :)

[elukey]

Anybody up for a review? It has been a long time :(

[mistercrunch]

iloc was introduced because pandas' behavior changed when dealing with multiple columns with the same key. Before it would return the first one, now it returns a dataframe which broke things.

Does this work when the same metric is used twice?

[elukey]

iloc was introduced because pandas' behavior changed when dealing with multiple columns with the same key. Before it would return the first one, now it returns a dataframe which broke things.

Does this work when the same metric is used twice?

I am running superset 0.32 with some extra patches, including this one, and charts using worlmap with the same metric reused as bubble size work fine now.

I have checked a couple of times the pandas' changelog to spot any new feature that could have changed its behavior, but found none. After checking git blame for WorldMapViz though, I noticed that the following bit in the commit that I have indicated seems now the responsible for the need of .iloc or not:

    def query_obj(self):
        qry = super(WorldMapViz, self).query_obj()
-        qry['metrics'] = [
-            self.form_data['metric'], self.form_data['secondary_metric']]
        qry['groupby'] = [self.form_data['entity']]
        return qry

[mistercrunch]

What version of pandas are you using? Current master says pandas==0.23.4

[elukey]

What version of pandas are you using? Current master says pandas==0.23.4

I am using 0.23.4 indeed, this is my pip freeze:

pip freeze | egrep 'pandas|numpy'
numpy==1.15.2
pandas==0.23.4

[elukey]

@mistercrunch there is more context in the related GH issue: #7006 (comment)

[elukey]

Any news? :)

[villebro]

@elukey I was able to replicate the problem on master, i.e. when choosing the same metric twice (color and bubble size), the error Too many indexers would show up. After applying your fix the problem went away without causing adverse side-effects that I could tell. If we want to be on the safe side, we could check if df[metric] is a DataFrame and call iloc on that. But I agree that there shouldn't be duplicate columns anymore, so it might very well just introduce unnecessary clutter into the codebase. So all in all LGTM.

[elukey]

@mistercrunch thoughts? :)