Skip to content

Commit

Permalink
More show improvements (#185)
Browse files Browse the repository at this point in the history
* bugfix missing or wrongly named variables

* bugfix show

* try fixing point show by adding kwarg

* Use sprint() instead of global IO buffers in test_display

* Test the printing of wrapped geometries

* we be rappin now

this is probably the best example of a 9pm commit I've had so far...

in all seriousness we could change the name

* Forward 2-arg show to 3-arg text/plain show

This allows the new show methods to also work in e.g arrays.

* `MIME("text/plain")` -> `MIME"text/plain"()`

fix typos

* wrap crs in quotes when showing

then, you can actually copy/paste the code

* Force display size to be consistent in test function

* Fix tests for new syntax

---------

Co-authored-by: Rafael Schouten <rafaelschouten@gmail.com>
  • Loading branch information
asinghvi17 and rafaqz authored Jan 20, 2025
1 parent d0e9ab3 commit 37b07f6
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 42 deletions.
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 @@ function ngeom(trait::AbstractGeometryTrait, geom::WrapperGeometry{<:Any,<:Any,T
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 @@ for (geomtype, trait, childtype, child_trait, length_check, nesting) in (
# 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 @@ for (geomtype, trait, childtype, child_trait, length_check, nesting) in (
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))"
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 @@ for (geomtype, trait, childtype, child_trait, length_check, nesting) in (

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)
end

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

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 @@ function Base.:(==)(g1::Point, g2::Point)
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 @@ function Base.show(io::IO, ::MIME"text/plain", point::Point{Z, M, T, C}; show_mz

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 @@ function Base.show(io::IO, ::MIME"text/plain", f::Feature; show_mz::Bool = true)
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 @@ function Base.show(io::IO, ::MIME"text/plain", f::Feature; show_mz::Bool = true)
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)
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 @@ function Base.show(io::IO, ::MIME"text/plain", fc::FeatureCollection)
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 @@ _child_feature_error() = throw(ArgumentError("child objects must be features"))
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 @@ crs(fc::FeatureCollection) =

_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))")

# 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

0 comments on commit 37b07f6

Please sign in to comment.