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

fix(maps): Move Overseas department and regions closer to France mainland #26995

Merged
merged 2 commits into from
Feb 3, 2024

Conversation

qleroy
Copy link
Contributor

@qleroy qleroy commented Feb 2, 2024

feat(maps): add drom closer to mainland France

SUMMARY

The notebook repositions the overseas departments and regions of France (DROM) closer to mainland.
The file france.geojson is generated from the notebook.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before (only France metropolitan departments):
france_before

After (DROM included):
france_after

TESTING INSTRUCTIONS

Execute the notebook to observe that France DROM are effectively moved closer to mainland so that it can be display in a single map.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@qleroy
Copy link
Contributor Author

qleroy commented Feb 2, 2024

The relevant bit of code because there is no diff for notebook on github

france_copy = france.copy()
reposition(france_copy, france.name=='Guadeloupe', 57.4, 25.4, 1.5, 1.5)
reposition(france_copy, france.name=='Martinique', 58.4, 27.1, 1.5, 1.5)
reposition(france_copy, france.name=='Guyane française', 52, 37.7, 0.35, 0.35)
reposition(france_copy, france.name=='La Réunion', -55, 62.8, 1.5, 1.5)
reposition(france_copy, france.name=='Mayotte', -43, 54.3, 1.5, 1.5)

@qleroy
Copy link
Contributor Author

qleroy commented Feb 2, 2024

I also fixed two metropolitan names (errors natural earth data),
that were fixed by hand in the superset repo in #22999 so a re-run of the notebook would have overwritten the fix.

  • Seien-et-Marne => Seine-et-Marne
  • Haute-Rhin => Haut-Rhin
 def replace_name(df, old, new):
    if old in list(df.name):  
        index = df[df.name == old].index[0]
        df.at[index, 'name'] = new
replace_name(france, 'Seien-et-Marne', 'Seine-et-Marne')
replace_name(france, 'Haute-Rhin', 'Haut-Rhin')

@rusackas rusackas changed the title Move Overseas department and regions closer to France mainland fix(maps): Move Overseas department and regions closer to France mainland Feb 2, 2024
@rusackas
Copy link
Member

rusackas commented Feb 2, 2024

This is awesome! So happy we're fixing maps the right way now!

Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (db79c37) 67.20% compared to head (866eb9a) 67.20%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #26995   +/-   ##
=======================================
  Coverage   67.20%   67.20%           
=======================================
  Files        1899     1899           
  Lines       74346    74346           
  Branches     8263     8263           
=======================================
  Hits        49963    49963           
  Misses      22334    22334           
  Partials     2049     2049           
Flag Coverage Δ
javascript 56.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qleroy
Copy link
Contributor Author

qleroy commented Feb 2, 2024

With the geopandas dissolve function,
I can get the France regions from the France departments !,
bypassing the resort to external sources like IGN like it was done in #25676 and #26848

we would have france_regions.geojson generated by the notebook !

@rusackas do I include this in this PR or in a separate one ?

Screenshot 2024-02-02 at 19 11 01
france_regions = france.copy()
# source : https://www.iso.org/obp/ui/#iso:code:3166:FR
dep_to_reg = {
"FR-01": "FR-ARA",
"FR-02": "FR-HDF",
"FR-03": "FR-ARA",
"FR-04": "FR-PAC",
"FR-05": "FR-PAC",
"FR-06": "FR-PAC",
"FR-07": "FR-ARA",
"FR-08": "FR-GES",
"FR-09": "FR-OCC",
"FR-10": "FR-GES",
"FR-11": "FR-OCC",
"FR-12": "FR-OCC",
"FR-13": "FR-PAC",
"FR-14": "FR-NOR",
"FR-15": "FR-ARA",
"FR-16": "FR-NAQ",
"FR-17": "FR-NAQ",
"FR-18": "FR-CVL",
"FR-19": "FR-NAQ",
"FR-21": "FR-BFC",
"FR-22": "FR-BRE",
"FR-23": "FR-NAQ",
"FR-24": "FR-NAQ",
"FR-25": "FR-BFC",
"FR-26": "FR-ARA",
"FR-27": "FR-NOR",
"FR-28": "FR-CVL",
"FR-29": "FR-BRE",
"FR-2A": "FR-20R",
"FR-2B": "FR-20R",
"FR-30": "FR-OCC",
"FR-31": "FR-OCC",
"FR-32": "FR-OCC",
"FR-33": "FR-NAQ",
"FR-34": "FR-OCC",
"FR-35": "FR-BRE",
"FR-36": "FR-CVL",
"FR-37": "FR-CVL",
"FR-38": "FR-ARA",
"FR-39": "FR-BFC",
"FR-40": "FR-NAQ",
"FR-41": "FR-CVL",
"FR-42": "FR-ARA",
"FR-43": "FR-ARA",
"FR-44": "FR-PDL",
"FR-45": "FR-CVL",
"FR-46": "FR-OCC",
"FR-47": "FR-NAQ",
"FR-48": "FR-OCC",
"FR-49": "FR-PDL",
"FR-50": "FR-NOR",
"FR-51": "FR-GES",
"FR-52": "FR-GES",
"FR-53": "FR-PDL",
"FR-54": "FR-GES",
"FR-55": "FR-GES",
"FR-56": "FR-BRE",
"FR-57": "FR-GES",
"FR-58": "FR-BFC",
"FR-59": "FR-HDF",
"FR-60": "FR-HDF",
"FR-61": "FR-NOR",
"FR-62": "FR-HDF",
"FR-63": "FR-ARA",
"FR-64": "FR-NAQ",
"FR-65": "FR-OCC",
"FR-66": "FR-OCC",
"FR-67": "FR-GES",
"FR-68": "FR-GES",
"FR-69": "FR-ARA",
"FR-70": "FR-BFC",
"FR-71": "FR-BFC",
"FR-72": "FR-PDL",
"FR-73": "FR-ARA",
"FR-74": "FR-ARA",
"FR-75": "FR-IDF",
"FR-76": "FR-NOR",
"FR-77": "FR-IDF",
"FR-78": "FR-IDF",
"FR-79": "FR-NAQ",
"FR-80": "FR-HDF",
"FR-81": "FR-OCC",
"FR-82": "FR-OCC",
"FR-83": "FR-PAC",
"FR-84": "FR-PAC",
"FR-85": "FR-PDL",
"FR-86": "FR-NAQ",
"FR-87": "FR-NAQ",
"FR-88": "FR-GES",
"FR-89": "FR-BFC",
"FR-90": "FR-BFC",
"FR-91": "FR-IDF",
"FR-92": "FR-IDF",
"FR-93": "FR-IDF",
"FR-94": "FR-IDF",
"FR-95": "FR-IDF",
"FR-GF": "FR-GF",
"FR-MQ": "FR-MQ",
"FR-GP": "FR-GP",
"FR-RE": "FR-RE",
"FR-YT": "FR-YT",
}
france_regions['iso_3166_2_fr'] = france_regions['iso_3166_2'].apply(lambda x: dep_to_reg[x])
reposition(france_regions, france.name=='Guadeloupe', 57.4, 25.4, 1.5, 1.5)
reposition(france_regions, france.name=='Martinique', 58.4, 27.1, 1.5, 1.5)
reposition(france_regions, france.name=='Guyane française', 52, 37.7, 0.35, 0.35)
reposition(france_regions, france.name=='La Réunion', -55, 62.8, 1.5, 1.5)
reposition(france_regions, france.name=='Mayotte', -43, 54.3, 1.5, 1.5)
france_regions = france_regions.dissolve(by='iso_3166_2_reg')
france_regions['iso_3166_2'] = france_regions['iso_3166_2'].index
france_regions.plot()

@john-bodley john-bodley added the review:checkpoint Last PR reviewed during the daily review standup label Feb 2, 2024
Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

LGTM

@rusackas
Copy link
Member

rusackas commented Feb 3, 2024

@rusackas do I include this in this PR or in a separate one ?

A separate one would be ideal, I think :) Then after I see how you sorted that out, maybe one of us can do the same for the Italy Regions... then there will be no exceptions!

Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Finally got a chance to pull and run the notebook... everything looks great!

@rusackas rusackas merged commit 2602527 into apache:master Feb 3, 2024
34 of 36 checks passed
@qleroy
Copy link
Contributor Author

qleroy commented Feb 5, 2024

@rusackas I did the follow up here #27014

so we have only countries_custom/italy_regions.geojson remaining

@rusackas rusackas removed the review:checkpoint Last PR reviewed during the daily review standup label Feb 5, 2024
sfirke pushed a commit to sfirke/superset that referenced this pull request Mar 22, 2024
@ThomasG77
Copy link

FYI, similar recipe using mapshaper (pure CLI approach) https://gist.github.com/ThomasG77/e20c52a39c581bab6a9e42e87d15b9e2 (in French, can translate if needed).

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 4.0.0 labels Apr 17, 2024
vinothkumar66 pushed a commit to vinothkumar66/superset that referenced this pull request Nov 11, 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 size/L 🚢 4.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants