-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
feat(maps): Consolidating all country maps (and TS) into the Jupyter notebook workflow. #26300
Conversation
…s in the notebook, and auto-generated TS
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26300 +/- ##
==========================================
- Coverage 69.18% 69.17% -0.01%
==========================================
Files 1946 1949 +3
Lines 75988 80251 +4263
Branches 8479 9732 +1253
==========================================
+ Hits 52570 55514 +2944
- Misses 21228 22320 +1092
- Partials 2190 2417 +227
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/testenv up |
@rusackas Ephemeral environment spinning up at http://52.37.45.250:8080. Credentials are |
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.
As a Finn this caught my eye - apparently Åland Islands
has been replaced by Åland
in the dataset. Can you replace the following snippet:
finland_aland = df_admin0_10m.loc[
df_admin0_10m.name_en.isin(['Åland Islands']),
[x for x in df_admin0_10m.columns if x in df.columns]
]
finland_aland = finland_aland.merge(pd.DataFrame(
data={
"name_en": ["Åland Islands"],
"name_fi": ["Ahvenanmaan maakunta"],
"iso_3166_2": ["FI-01"],
},
), on="name_en", how="left")
with
finland_aland = df_admin0_10m.loc[
df_admin0_10m.name_en.isin(['Åland']),
[x for x in df_admin0_10m.columns if x in df.columns]
]
finland_aland = finland_aland.merge(pd.DataFrame(
data={
"name_en": ["Åland"],
"name_fi": ["Ahvenanmaan maakunta"],
"iso_3166_2": ["FI-01"],
},
), on="name_en", how="left")
This will ensure that the Finnish southwestern archipelago looks correct.
@villebro done! |
To consolidate the geojson generation with the notebook, This could also be a step in the documentation to note that it is required to provide files projected with WGS84 |
Also, I would love that we include overseas islands for France ! |
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.
LGTM!
@qleroy would you be open to contributing the floating islands for the French maps? Also the other improvement ideas you presented. That would be awesome! 🇫🇷 |
Happy to keep iterating on this, and indeed would love help with any of it! Meanwhile, let's merge this swath of it, so others can fork and contribute :) Thanks! |
Ephemeral environment shutdown and build artifacts deleted. |
Love it! Thanks for the follow-through @qleroy |
…notebook workflow. (apache#26300)
Hi all, thanks! |
…notebook workflow. (apache#26300)
SUMMARY
Ok, so our little world of maps is a a heterogenous mess :) Time to clean house! This PR does the following:
france_regions
map seems to be causing a browser meltdown before or after this PR. I'm not sure what's up with that.TODO (in this PR or subsequent ones)
• Figure out what's broken with the france_regions map
• Add documentation about this notebook to the CONTRIBUTING file
• Add codeowners to the relevant files to prevent more diverging maps
• Add more countries!
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
I built a dataset of every airport in the world, and went through most, if not all, of the new maps. All work well except Sri Lanka, which I haven't gotten working with any dataset... there's something strange about their region codes.
ADDITIONAL INFORMATION