Skip to content

Add tests for tempfile_from_geojson() #1354

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

Closed
willschlitzer wants to merge 3 commits into from
+33 −0
Closed

Add tests for tempfile_from_geojson() #1354

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96dfc1b
add test_tempfile_from_geojson() and test_tempfile_from_geojson_no_ge…
willschlitzer Jun 23, 2021
645570d
run make format
willschlitzer Jun 23, 2021
c68b4f1
change assert statements to use isInstance
willschlitzer Jun 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions pygmt/tests/test_helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@
"""
import os

import geopandas as gpd
import numpy as np
import pytest
import shapely.geometry
from pygmt.exceptions import GMTInvalidInput
from pygmt.helpers import (
GMTTempFile,
args_in_kwargs,
data_kind,
kwargs_to_strings,
tempfile_from_geojson,
unique_name,
)

Expand Down Expand Up @@ -113,3 +116,33 @@ def test_args_in_kwargs():
# Failing list of arguments
failing_args = ["D", "E", "F"]
assert not args_in_kwargs(args=failing_args, kwargs=kwargs)


def test_tempfile_from_geojson():
"""
Test tempfile_from_geojson works when passed a geopandas GeoDataFrame.
"""
linestring = shapely.geometry.LineString([(20, 15), (30, 15)])
polygon = shapely.geometry.Polygon([(20, 10), (23, 10), (23, 14), (20, 14)])
gdf = gpd.GeoDataFrame(
index=["polygon", "linestring"],
geometry=[polygon, linestring],
)
with tempfile_from_geojson(gdf) as geojson_string:
assert isinstance(geojson_string, str)
with open(geojson_string) as tmpfile:
assert os.path.basename(tmpfile.name).startswith("pygmt-")
assert os.path.basename(tmpfile.name).endswith(".gmt")
Comment on lines +121 to +135
Copy link
Member

@weiji14 weiji14 Jun 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This is failing tests on the Python 3.7 CI jobs with ModuleNotFoundError: No module named 'geopandas'. Should geopandas be added/not made optional to requirements.txt and environment.yml?

No, geopandas should be an optional-only dependency. Could you provide some context as to why these two tests for the tempfile_from_geojson() function are to be added? To be honest, this tempfile_from_geojson function (added in #1000) is meant to be a temporary solution until GMT can read GeoJSON directly (as per GenericMappingTools/gmt#4599) though that might take some time.

If you must though, I'd suggest using pytest.importorskip to load geopandas and shapely, similar to at

pygmt/pygmt/tests/test_geopandas.py

Lines 8 to 9 in b2808cf

gpd = pytest.importorskip("geopandas")
shapely = pytest.importorskip("shapely")

Copy link
Contributor Author

@willschlitzer willschlitzer Jun 28, 2021

Choose a reason for hiding this comment

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

The reason I added it was that the code coverage was showing tempfile_from_geojson() wasn't tested. I just assumed we hadn't gotten around to writing tests for it yet so I wrote them.

Copy link
Member

@weiji14 weiji14 Jun 28, 2021

Choose a reason for hiding this comment

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

The tempfile_from_geojson function does have full test coverage (see https://codecov.io/gh/GenericMappingTools/pygmt/src/master/pygmt/helpers/tempfile.py). It is tested indirectly at test_geopandas.py.

Copy link
Contributor Author

@willschlitzer willschlitzer Jun 29, 2021

Choose a reason for hiding this comment

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

Sounds good, I'll close the PR.



def test_tempfile_from_geojson_no_geodataframe():
"""
Test tempfile_from_geojson when passed data not in a geopandas GeoDataFrame
format.
"""
linestring = shapely.geometry.LineString([(20, 15), (30, 15)])
with tempfile_from_geojson(linestring) as geojson_string:
assert isinstance(geojson_string, str)
with open(geojson_string) as tmpfile:
assert os.path.basename(tmpfile.name).startswith("pygmt-")
assert os.path.basename(tmpfile.name).endswith(".gmt")