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

Add gallery example showing usage of polygon objects from a geopandas.GeoDataFrame (choropleth map) #2796

Merged
merged 19 commits into from
Nov 10, 2023

Conversation

michaelgrund
Copy link
Member

@michaelgrund michaelgrund commented Nov 3, 2023

Description of proposed changes

Here's the second gallery example showcasing how to plot objects stored in a geopandas.GeoDataFrame, now for polygons.

Related to #1374, #1474, #2786, #2798.

Preview: https://pygmt-dev--2796.org.readthedocs.build/en/2796/gallery/maps/choropleth_map.html

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

@michaelgrund michaelgrund added the documentation Improvements or additions to documentation label Nov 3, 2023
@michaelgrund michaelgrund added this to the 0.11.0 milestone Nov 3, 2023
@michaelgrund
Copy link
Member Author

/format

@michaelgrund michaelgrund added the needs review This PR has higher priority and needs review. label Nov 3, 2023
examples/gallery/lines/polygons.py Outdated Show resolved Hide resolved
examples/gallery/lines/polygons.py Outdated Show resolved Hide resolved
examples/gallery/lines/polygons.py Outdated Show resolved Hide resolved
michaelgrund and others added 2 commits November 3, 2023 15:01
Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
examples/gallery/lines/polygons.py Outdated Show resolved Hide resolved
examples/gallery/lines/polygons.py Outdated Show resolved Hide resolved
examples/gallery/lines/choropleth_map.py Outdated Show resolved Hide resolved
examples/gallery/lines/choropleth_map.py Outdated Show resolved Hide resolved
examples/gallery/lines/choropleth_map.py Outdated Show resolved Hide resolved
examples/gallery/lines/choropleth_map.py Outdated Show resolved Hide resolved
@yvonnefroehlich
Copy link
Member

Maybe it makes more sense to place this example in the "maps" folder instead of the "lines" folder? Probably it is currently stored in the "lines" folder, as there is also the example for the line geometries. But this example is not really about lines.

michaelgrund and others added 2 commits November 6, 2023 10:52
@michaelgrund
Copy link
Member Author

Maybe it makes more sense to place this example in the "maps" folder instead of the "lines" folder? Probably it is currently stored in the "lines" folder, as there is also the example for the line geometries. But this example is not really about lines.

Done!

@michaelgrund
Copy link
Member Author

Will set this PR on draft until #2798 is merged.

@seisman
Copy link
Member

seisman commented Nov 6, 2023

Will set this PR on draft until #2798 is merged.

@michaelgrund I don't think I have time finishing PR #2798 in the next few weeks or one month. So it's up to you if you want (1) wait until PR #2798 is merged; or (2) merge this PR first, then simplify it in the future after #2798 is merged.

@michaelgrund
Copy link
Member Author

Will set this PR on draft until #2798 is merged.

@michaelgrund I don't think I have time finishing PR #2798 in the next few weeks or one month. So it's up to you if you want (1) wait until PR #2798 is merged; or (2) merge this PR first, then simplify it in the future after #2798 is merged.

Then let's go with option (2) and simplify it later.

fig.plot(
data=gdf,
pen="0.3p,gray10",
close=True,
Copy link
Member

@yvonnefroehlich yvonnefroehlich Nov 7, 2023

Choose a reason for hiding this comment

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

I feel we should explain that the close parameter is used to force closed polygons; maybe already in the introduction text at lines 12-14.

Copy link
Member

Choose a reason for hiding this comment

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

Is close required?

Copy link
Member

@yvonnefroehlich yvonnefroehlich Nov 8, 2023

Choose a reason for hiding this comment

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

This is something I am not 100 % sure about. I orientated myself on the EGU short course example for a choropleth map (https://www.generic-mapping-tools.org/egu22pygmt/ecosystem.html#plotting-geospatial-vector-data-with-geopandas-and-pygmt). And there, the close parameter is set to True to force closed polygons. I already tried it without using this, and the figure looked correct to me.

Copy link
Member

Choose a reason for hiding this comment

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

When plotting polygons without fills, close is required to connect the first and last points of polygons.

When filling polygons, the polygons are always "closed" by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

Then I would suggest to remove it in the example.

Copy link
Member

Choose a reason for hiding this comment

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

Hm. Based on this, it seems to me, we do not need to explicitly set the close parameter to True when creating a choropleth map from polygons provided via a GeoDataFrame.

I just tried to plot the polygons of this GeoDataFrame without any fill. Also this works for me correctly without using close=True:

import geopandas as gpd
import pygmt

gdf = gpd.read_file("https://geodacenter.github.io/data-and-lab/data/airbnb.zip")
fig = pygmt.Figure()

fig.basemap(region=gdf.total_bounds[[0, 2, 1, 3]], projection="M6c", frame="+t ")
fig.plot(data=gdf)

fig.show()

Output figure
geodataframe_withoutfill

Copy link
Member

@seisman seisman Nov 9, 2023

Choose a reason for hiding this comment

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

Here is an example showing how close works with/without fill.

import pygmt
fig = pygmt.Figure()
data = [[1, 1], [1, 5], [5, 5], [5, 1]]
fig.plot(data=data, projection="X10c/10c", frame=True, region=[0, 10, 0, 10], pen="1p")
fig.shift_origin(xshift="w+1c")
fig.plot(data=data, projection="X10c/10c", frame=True, region=[0, 10, 0, 10], pen="1p", close=True)
fig.shift_origin(xshift="w+1c")
fig.plot(data=data, projection="X10c/10c", frame=True, region=[0, 10, 0, 10], pen="1p", fill="lightblue")
fig.show()

map

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @seisman!

Copy link
Member

Choose a reason for hiding this comment

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

I am wondering whether we should update the code example for the choropleth map of the EGU22 short course and remove the close parameter for consistency, despite the issue that the choropleth map is not displayed correctly. Interestingly, I still do not face this issue when running this Jupyter notebook locally in JupyterLab, neither within a conda environment based on the provided yaml file nor within a conda environment with PyGMT dev version and GMT 6.4.0 installed.

@seisman seisman added final review call This PR requires final review and approval from a second reviewer and removed needs review This PR has higher priority and needs review. labels Nov 8, 2023
@yvonnefroehlich
Copy link
Member

Maybe we should make the PR title more specific and include the term "chorolepth map"?

Copy link
Member

@yvonnefroehlich yvonnefroehlich left a comment

Choose a reason for hiding this comment

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

Beside the suggestions above, this example looks good to me!

Co-authored-by: Yvonne Fröhlich <94163266+yvonnefroehlich@users.noreply.github.com>
@michaelgrund michaelgrund changed the title Add gallery example showing usage of polygon objects from a geopandas.GeoDataFrame Add gallery example showing usage of polygon objects from a geopandas.GeoDataFrame (choropleth map) Nov 9, 2023
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Nov 10, 2023
@seisman seisman merged commit 10c434f into main Nov 10, 2023
8 checks passed
@seisman seisman deleted the add-gallery-polygons branch November 10, 2023 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants