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

More show improvements #185

Merged
merged 11 commits into from
Jan 20, 2025
51 changes: 31 additions & 20 deletions src/wrappers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,11 @@
isgeometry(T) ? ngeom(parent(geom)) : length(parent(geom))
end

# Forward all 2-argument show calls to the 3-argument version
# with a "text/plain" MIME type. This gives us a consistent target
# to implement the show methods.
Base.show(io::IO, geom::WrapperGeometry) = show(io, MIME"text/plain"(), geom)

# We define all the types in a loop so we have standardised docs and behaviour
# without too much repetition of code.
# `child_trait` and `child_type` define the trait and type of child geometries
Expand Down Expand Up @@ -172,7 +177,9 @@
# But not if geom is already a WrapperGeometry
convert(::Type{$geomtype}, ::$trait, geom::$geomtype) = geom

function Base.show(io::IO, ::MIME"text/plain", geom::$geomtype{Z, M, T, E, C}; show_mz::Bool = true, screen_ncols::Int = displaysize(io)[2]) where {Z, M, T, E <: Union{Nothing,Extents.Extent}, C}
function Base.show(io::IO, ::MIME"text/plain", geom::$geomtype{Z, M, T, E, C};
show_mz::Bool = true, screen_ncols::Int = displaysize(io)[2]
) where {Z, M, T, E <: Union{Nothing,Extents.Extent}, C}
compact = get(io, :compact, false)
spacing = compact ? "" : " "
show_mz &= !compact
Expand All @@ -181,14 +188,14 @@
crs_str = ""
if !compact
if !isnothing(geom.extent)
extent_str = ",$(spacing)extent$(spacing)=$(spacing)$(repr(MIME("text/plain"), geom.extent))"
extent_str = ",$(spacing)extent$(spacing)=$(spacing)$(repr(MIME"text/plain"(), geom.extent))"

Check warning on line 191 in src/wrappers.jl

View check run for this annotation

Codecov / codecov/patch

src/wrappers.jl#L191

Added line #L191 was not covered by tests
end
if !isnothing(geom.crs)
crs_str = ",$(spacing)crs$(spacing)=$(spacing)$(repr(MIME("text/plain"), geom.crs))"
crs_str = ",$(spacing)crs$(spacing)=$(spacing)\"$(repr(MIME"text/plain"(), geom.crs))\""
end
end

str = "$($geomtype)"
str = string($geomtype)
if show_mz
str *= "{$Z,$(spacing)$M}"
end
Expand Down Expand Up @@ -234,7 +241,7 @@

str *= "]"
else
str *= _nice_geom_str(g, false, compact, screen_ncols - currently_used_space)
str *= _nice_geom_str(parent(geom), false, compact, screen_ncols - currently_used_space)

Check warning on line 244 in src/wrappers.jl

View check run for this annotation

Codecov / codecov/patch

src/wrappers.jl#L244

Added line #L244 was not covered by tests
end

str *= extent_str
Expand Down Expand Up @@ -310,14 +317,14 @@

function _nice_geom_str(geom, ::Bool, ::Bool, ::Int)
io = IOBuffer()
show(io, MIME("text/plain"), geom)
show(io, MIME"text/plain"(), geom)
return String(take!(io))
end
# need a work around to pass the show_mz variable through - put string to a temp IOBuffer then read it
function _nice_geom_str(geom::WrapperGeometry, show_mz::Bool, compact::Bool, screen_ncols::Int)
buf = IOBuffer()
io = IOContext(IOContext(buf, :compact => compact))
show(io, MIME("text/plain"), geom; show_mz = show_mz, screen_ncols = screen_ncols)
show(io, MIME"text/plain"(), geom; show_mz = show_mz, screen_ncols = screen_ncols)
return String(take!(buf))
end
# handle tuples/vectors explicitly
Expand Down Expand Up @@ -449,7 +456,7 @@
end
Base.:(!=)(g1::Point, g2::Point) = !(g1 == g2)

function Base.show(io::IO, ::MIME"text/plain", point::Point{Z, M, T, C}; show_mz::Bool = true) where {Z,M,T,C}
function Base.show(io::IO, ::MIME"text/plain", point::Point{Z, M, T, C}; show_mz::Bool = true, screen_ncols::Int = displaysize(io)[2]) where {Z,M,T,C}
print(io, "Point")
this_crs = crs(point)

Expand All @@ -472,7 +479,9 @@

if !compact && !isnothing(this_crs)
print(io, ",$(spacing)crs$(spacing)=$(spacing)")
show(io, MIME("text/plain"), this_crs)
print(io, "\"")
show(io, MIME"text/plain"(), this_crs)
print(io, "\"")
end
print(io, ")")

Expand Down Expand Up @@ -527,7 +536,7 @@
compact = get(io, :compact, false)
spacing = compact ? "" : " "
print(io, "Feature(")
show(io, MIME("text/plain"), f.parent.geometry; show_mz = show_mz)
show(io, MIME"text/plain"(), f.parent.geometry; show_mz = show_mz)
non_geom_props = filter(!=(:geometry), propertynames(f.parent))
if !isempty(non_geom_props)
print(io, ",$(spacing)properties$(spacing)=$(spacing)(")
Expand All @@ -543,11 +552,12 @@
if !compact
if !isnothing(f.extent)
print(io, ", extent$(spacing)=$(spacing)")
show(io, MIME("text/plain"), f.extent)
show(io, MIME"text/plain"(), f.extent)

Check warning on line 555 in src/wrappers.jl

View check run for this annotation

Codecov / codecov/patch

src/wrappers.jl#L555

Added line #L555 was not covered by tests
end
if !isnothing(f.crs)
print(io, ", crs$(spacing)=$(spacing)")
show(io, MIME("text/plain"), f.crs)
print(io, ", crs$(spacing)=$(spacing)\"")
show(io, MIME"text/plain"(), f.crs)
print(io, "\"")
end
end
print(io, ")")
Expand Down Expand Up @@ -625,20 +635,21 @@
features = _parent_is_fc(fc) ? getfeature(trait(fc), parent(fc)) : parent(fc)
print(io, "[")
for (i, f) ∈ enumerate(features)
show(io, MIME("text/plain"), f; show_mz = !compact)
show(io, MIME"text/plain"(), f; show_mz = !compact)
if i != length(features)
print(io, ",$(spacing)")
end
end
print(io, "]")
if !compact
if !isnothing(fc.crs)
print(io, ",$(spacing)crs$(spacing)=$(spacing)")
show(io, MIME("text/plain"), fc.crs)
print(io, ",$(spacing)crs$(spacing)=$(spacing)\"")
show(io, MIME"text/plain"(), fc.crs)
print(io, "\"")
end
if !isnothing(fc.extent)
print(io, ",$(spacing)extent$(spacing)=$(spacing)")
show(io, MIME("text/plain"), fc.extent)
show(io, MIME"text/plain"(), fc.extent)
end
end
print(io, ")")
Expand All @@ -652,9 +663,9 @@
isfeaturecollection(fc::Type{<:FeatureCollection}) = true
trait(fc::FeatureCollection) = FeatureCollectionTrait()

nfeature(::FeatureCollectionTrait, fc::FeatureCollection) =
nfeature(t::FeatureCollectionTrait, fc::FeatureCollection) =
_parent_is_fc(fc) ? nfeature(t, parent(fc)) : length(parent(fc))
getfeature(::FeatureCollectionTrait, fc::FeatureCollection) =
getfeature(t::FeatureCollectionTrait, fc::FeatureCollection) =
_parent_is_fc(fc) ? getfeature(t, parent(fc)) : parent(fc)
getfeature(t::FeatureCollectionTrait, fc::FeatureCollection, i::Integer) =
_parent_is_fc(fc) ? getfeature(t, parent(fc), i) : parent(fc)[i]
Expand All @@ -665,4 +676,4 @@

_parent_is_fc(x) = isfeaturecollection(parent(x))

end # module
end # module
72 changes: 50 additions & 22 deletions test/test_wrappers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@ using Test, GeoFormatTypes, Extents
import GeoInterface as GI
using GeoInterface.Wrappers

# use this to test our string representations for geoms
buf = IOBuffer()
compact_buf = IOContext(buf, :compact => true)

# checks that our string display for geoms in regular/compact form is as expected
function test_display(geom, expected_str, expected_compact_str)
# checks non-compact string repr
show(buf, MIME"text/plain"(), geom)
@test expected_str == String(take!(buf))
generated_str = sprint() do io
show(IOContext(io, :displaysize => (24, 80)), MIME"text/plain"(), geom)
end
@test expected_str == generated_str
# checks compact string repr
show(compact_buf, MIME"text/plain"(), geom)
@test expected_compact_str == String(take!(buf))
generated_compact_str = sprint() do io
show(IOContext(io, :displaysize => (24, 80), :compact => true), MIME"text/plain"(), geom)
end
@test expected_compact_str == generated_compact_str
end

# Point
Expand All @@ -35,7 +35,7 @@ test_display(point, "Point{false, false}((1, 2))", "Point((1,2))")
point_crs = GI.Point(point; crs=EPSG(4326))
@test parent(point_crs) === parent(point)
@test GI.crs(point_crs) === EPSG(4326)
test_display(point_crs, "Point{false, false}((1, 2), crs = EPSG{1}((4326,)))", "Point((1,2))")
test_display(point_crs, "Point{false, false}((1, 2), crs = \"EPSG:4326\")", "Point((1,2))")

# 3D Point
pointz = GI.Point(1, 2, 3)
Expand Down Expand Up @@ -80,7 +80,7 @@ test_display(pointm, "Point{false, true}((1, 2, 3))", "Point((1,2,3))")
pointm_crs = GI.Point((X=1, Y=2, M=3); crs=EPSG(4326))
@test parent(pointm_crs) === parent(pointm)
@test GI.crs(pointm_crs) === EPSG(4326)
test_display(pointm_crs, "Point{false, true}((1, 2, 3), crs = EPSG{1}((4326,)))", "Point((1,2,3))")
test_display(pointm_crs, "Point{false, true}((1, 2, 3), crs = \"EPSG:4326\")", "Point((1,2,3))")
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if showing EPSG as a string is confusing to users who still need to use the constructor

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, but it's equivalent in most cases that one might care about (Proj and GDAL)

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should change show on GFT types to show the constructor too?


# Forced measured point with a tuple
pointtm = GI.Point{false,true}(1, 2, 3)
Expand All @@ -95,7 +95,7 @@ test_display(pointtm, "Point{false, true}((1, 2, 3))", "Point((1,2,3))")
pointtm_crs = GI.Point{false,true}(1, 2, 3; crs=EPSG(4326))
@test parent(pointtm_crs) === parent(pointtm)
@test GI.crs(pointtm_crs) === EPSG(4326)
test_display(pointtm_crs, "Point{false, true}((1, 2, 3), crs = EPSG{1}((4326,)))", "Point((1,2,3))")
test_display(pointtm_crs, "Point{false, true}((1, 2, 3), crs = \"EPSG:4326\")", "Point((1,2,3))")

# Point made from an array
pointa = GI.Point([1, 2])
Expand Down Expand Up @@ -149,7 +149,7 @@ test_display(line, "Line{false, false}([(1, 2), (3, 4)])", "Line([(1,2),(3,4)])"
line_crs = GI.Line(line; crs=EPSG(4326))
@test parent(line_crs) === parent(line)
@test GI.crs(line_crs) === EPSG(4326)
test_display(line_crs, "Line{false, false}([(1, 2), (3, 4)], crs = EPSG{1}((4326,)))", "Line([(1,2),(3,4)])")
test_display(line_crs, "Line{false, false}([(1, 2), (3, 4)], crs = \"EPSG:4326\")", "Line([(1,2),(3,4)])")

# LineString
linestring = GI.LineString([(1, 2), (3, 4)])
Expand All @@ -165,7 +165,7 @@ test_display(linestring, "LineString{false, false}([(1, 2), (3, 4)])", "LineStri
linestring_crs = GI.LineString(linestring; crs=EPSG(4326))
@test parent(linestring_crs) === parent(linestring)
@test GI.crs(linestring_crs) === EPSG(4326)
test_display(linestring_crs, "LineString{false, false}([(1, 2), (3, 4)], crs = EPSG{1}((4326,)))", "LineString([(1,2),(3,4)])")
test_display(linestring_crs, "LineString{false, false}([(1, 2), (3, 4)], crs = \"EPSG:4326\")", "LineString([(1,2),(3,4)])")

# LinearRing
linearring = GI.LinearRing([(1, 2), (3, 4), (5, 6), (1, 2)])
Expand All @@ -181,7 +181,7 @@ test_display(linearring, "LinearRing{false, false}([(1, 2), (3, 4), (5, 6), (1,
linearring_crs = GI.LinearRing(linearring; crs=EPSG(4326))
@test parent(linearring_crs) === parent(linearring)
@test GI.crs(linearring_crs) === EPSG(4326)
test_display(linearring_crs, "LinearRing{false, false}([(1, 2), (3, 4), (5, 6), (1, 2)], crs = EPSG{1}((4326,)))", "LinearRing([(1,2),(3,4),(5,6),(1,2)])")
test_display(linearring_crs, "LinearRing{false, false}([(1, 2), (3, 4), (5, 6), (1, 2)], crs = \"EPSG:4326\")", "LinearRing([(1,2),(3,4),(5,6),(1,2)])")

# Polygon
polygon = GI.Polygon([linearring, linearring])
Expand All @@ -201,7 +201,7 @@ test_display(polygon, "Polygon{false, false}([LinearRing([(1, 2), … (2) … ,
polygon_crs = GI.Polygon(polygon; crs=EPSG(4326))
@test parent(polygon_crs) === parent(polygon)
@test GI.crs(polygon_crs) === EPSG(4326)
test_display(polygon_crs, "Polygon{false, false}([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])], crs = EPSG{1}((4326,)))",
test_display(polygon_crs, "Polygon{false, false}([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])], crs = \"EPSG:4326\")",
"Polygon([LinearRing([(1,2),(3,4),(5,6),(1,2)]),LinearRing([(1,2),(3,4),(5,6),(1,2)])])")
# Make sure `linestring` is also ok in polygons
polygon = GI.Polygon([linestring, linestring])
Expand Down Expand Up @@ -231,7 +231,7 @@ test_display(multipoint, "MultiPoint{false, false}([(1, 2), (3, 4), (3, 2), (1,
multipoint_crs = GI.MultiPoint(multipoint; crs=EPSG(4326))
@test parent(multipoint_crs) == parent(multipoint)
@test GI.crs(multipoint_crs) === EPSG(4326)
test_display(multipoint_crs, "MultiPoint{false, false}([(1, 2), (3, 4), … (2) … , (7, 8), (9, 10)], crs = EPSG{1}((4326,)))", "MultiPoint([(1,2),(3,4),(3,2),(1,4),(7,8),(9,10)])")
test_display(multipoint_crs, "MultiPoint{false, false}([(1, 2), (3, 4), (3, 2), … (1) … , (7, 8), (9, 10)], crs = \"EPSG:4326\")", "MultiPoint([(1,2),(3,4),(3,2),(1,4),(7,8),(9,10)])")

# GeometryCollection
geoms = [line, linestring, linearring, multipoint, (1, 2)]
Expand All @@ -248,7 +248,7 @@ test_display(collection, "GeometryCollection{false, false}([Line([(1, 2), (3, 4)
collection_crs = GI.GeometryCollection(collection; crs=EPSG(4326))
@test parent(collection_crs) == parent(collection)
@test GI.crs(collection_crs) === EPSG(4326)
test_display(collection_crs, "GeometryCollection{false, false}([Line([(1, 2), (3, 4)]), … (3) … , (1, 2)], crs = EPSG{1}((4326,)))",
test_display(collection_crs, "GeometryCollection{false, false}([Line([(1, 2), (3, 4)]), … (3) … , (1, 2)], crs = \"EPSG:4326\")",
"GeometryCollection([Line([(1,2),(3,4)]),LineString([(1,2),(3,4)]),…(2)…,(1,2)])")

# MultiCurve
Expand All @@ -267,7 +267,7 @@ test_display(multicurve, "MultiCurve{false, false}([LineString([(1, 2), (3, 4)])
multicurve_crs = GI.MultiCurve(multicurve; crs=EPSG(4326))
@test parent(multicurve_crs) == parent(multicurve)
@test GI.crs(multicurve_crs) === EPSG(4326)
test_display(multicurve_crs, "MultiCurve{false, false}([LineString([(1, 2), (3, 4)]), LinearRing([(1, 2), … (2) … , (1, 2)])], crs = EPSG{1}((4326,)))",
test_display(multicurve_crs, "MultiCurve{false, false}([LineString([(1, 2), (3, 4)]), LinearRing([(1, 2), … (2) … , (1, 2)])], crs = \"EPSG:4326\")",
"MultiCurve([LineString([(1,2),(3,4)]),LinearRing([(1,2),(3,4),…(1)…,(1,2)])])")

# MultiPolygon
Expand All @@ -288,7 +288,7 @@ test_display(multipolygon, "MultiPolygon{false, false}([Polygon([LinearRing([(1,
multipolygon_crs = GI.MultiPolygon(multipolygon; crs=EPSG(4326))
@test parent(multipolygon_crs) == parent(multipolygon)
@test GI.crs(multipolygon_crs) === EPSG(4326)
test_display(multipolygon_crs, "MultiPolygon{false, false}([Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])])], crs = EPSG{1}((4326,)))",
test_display(multipolygon_crs, "MultiPolygon{false, false}([Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])])], crs = \"EPSG:4326\")",
"MultiPolygon([Polygon([LinearRing([(1,2),…(2)…,(1,2)]),LinearRing([(1,2),…(2)…,(1,2)])])])")

# PolyhedralSurface
Expand All @@ -308,7 +308,7 @@ test_display(polyhedralsurface, "PolyhedralSurface{false, false}([Polygon([Linea
polyhedralsurface_crs = GI.PolyhedralSurface(polyhedralsurface; crs=EPSG(4326))
@test parent(polyhedralsurface_crs) == parent(polyhedralsurface)
@test GI.crs(polyhedralsurface_crs) === EPSG(4326)
test_display(polyhedralsurface_crs, "PolyhedralSurface{false, false}([Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])]), Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])])], crs = EPSG{1}((4326,)))",
test_display(polyhedralsurface_crs, "PolyhedralSurface{false, false}([Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])]), Polygon([LinearRing([(1, 2), … (2) … , (1, 2)]), LinearRing([(1, 2), … (2) … , (1, 2)])])], crs = \"EPSG:4326\")",
"PolyhedralSurface([Polygon([LinearRing([(1,2),…(2)…,(1,2)]),LinearRing([(1,2),…(2)…,(1,2)])]),Polygon([LinearRing([(1,2),…(2)…,(1,2)]),LinearRing([(1,2),…(2)…,(1,2)])])])")

# Round-trip coordinates
Expand Down Expand Up @@ -338,7 +338,7 @@ test_display(feature, "Feature(MultiPolygon{false, false}([Polygon([LinearRing([
feature = GI.Feature(multipolygon;
properties=(x=1, y=2, z=3), crs=EPSG(4326), extent=extent(multipolygon)
)
test_display(feature, "Feature(MultiPolygon{false, false}([Polygon([LinearRing([[1, 2], [3, 4], [3, 2], [1, 4]])])]), properties = (x = 1, y = 2, z = 3), crs = EPSG{1}((4326,)))",
test_display(feature, "Feature(MultiPolygon{false, false}([Polygon([LinearRing([[1, 2], [3, 4], [3, 2], [1, 4]])])]), properties = (x = 1, y = 2, z = 3), crs = \"EPSG:4326\")",
"Feature(MultiPolygon([Polygon([LinearRing([[1,2],[3,4],[3,2],[1,4]])])]),properties=(x=1,y=2,z=3))")
@test GI.geometry(feature) === multipolygon
@test GI.properties(feature) === (x=1, y=2, z=3)
Expand All @@ -359,12 +359,40 @@ fc = GI.FeatureCollection(fc_unwrapped.parent; crs=EPSG(4326), extent=GI.extent(
@test GI.extent(fc) == fc.extent
@test first(GI.getfeature(fc)) == GI.getfeature(fc, 1) === feature
@test GI.testfeaturecollection(fc)
test_display(fc, "FeatureCollection([Feature(MultiPolygon{false, false}([Polygon([LinearRing([[1, 2], [3, 4], [3, 2], [1, 4]])])]), properties = (x = 1, y = 2, z = 3), crs = EPSG{1}((4326,)))], crs = EPSG{1}((4326,)), extent = Extent(X = (1, 3), Y = (2, 4)))",
test_display(fc, "FeatureCollection([Feature(MultiPolygon{false, false}([Polygon([LinearRing([[1, 2], [3, 4], [3, 2], [1, 4]])])]), properties = (x = 1, y = 2, z = 3), crs = \"EPSG:4326\")], crs = \"EPSG:4326\", extent = Extent(X = (1, 3), Y = (2, 4)))",
"FeatureCollection([Feature(MultiPolygon([Polygon([LinearRing([[1,2],[3,4],[3,2],[1,4]])])]),properties=(x=1,y=2,z=3))])")
@test_throws ArgumentError GI.FeatureCollection([1])
vecfc = GI.FeatureCollection([(geometry=(1,2), a=1, b=2)])
@test GI.getfeature(vecfc, 1) == (geometry=(1,2), a=1, b=2)



struct MaPointRappa
x::Float64
y::Float64
end

@testset "Wrapped geometry printing" begin

GI.geomtrait(::MaPointRappa) = GI.PointTrait()
GI.ncoord(::GI.PointTrait, ::MaPointRappa) = 2
GI.x(::GI.PointTrait, p::MaPointRappa) = p.x
GI.y(::GI.PointTrait, p::MaPointRappa) = p.y


test_display(GI.Point(MaPointRappa(1.0, 2.0)), "Point{false, false}((1.0, 2.0))", "Point((1.0,2.0))")

GI.geomtrait(::Vector{MaPointRappa}) = GI.LineStringTrait()
GI.npoint(::GI.LineStringTrait, v::Vector{MaPointRappa}) = length(v)
GI.getpoint(::GI.LineStringTrait, v::Vector{MaPointRappa}, i::Integer) = v[i]

test_display(
GI.LineString([MaPointRappa(1.0, 2.0), MaPointRappa(3.0, 4.0)]),
"LineString{false, false}([MaPointRappa(1.0, 2.0), MaPointRappa(3.0, 4.0)])",
"LineString([MaPointRappa(1.0, 2.0),MaPointRappa(3.0, 4.0)])" # FIXME: this should not show the point type!
)
end

# TODO

# # Triangle
Expand Down
Loading