Skip to content

Conversation

@rafaqz
Copy link
Member

@rafaqz rafaqz commented Jan 6, 2025

Fixes #176

Also fixes a few other missing variables after running JET

@evetion
Copy link
Member

evetion commented Jan 8, 2025

Tests??

@rafaqz
Copy link
Member Author

rafaqz commented Jan 8, 2025

Yeah

@asinghvi17
Copy link
Member

Looks like the new show methods don't catch everything:

julia> @show lr;
lr = GeoInterface.Wrappers.LinearRing{false, false, Vector{Tuple{Float64, Float64}}, Nothing, Nothing}([(0.4083621168668843, 0.3241081369331741), (0.6576490657994901, 0.8947885368160798), (0.28505143942419986, 0.33026739871915556), (0.6447348618937504, 0.45342604213989457), (0.21373432697077166, 0.44230337294222666), (0.10026772922087546, 0.6251846474866037), (0.9483433569917994, 0.7063875201028693), (0.707534681160356, 0.5215473041017257), (0.6162901701134593, 0.6319947001700302), (0.4083621168668843, 0.3241081369331741)], nothing, nothing)

@asinghvi17
Copy link
Member

though maybe that's two-arg show...

@rafaqz
Copy link
Member Author

rafaqz commented Jan 13, 2025

Should we also change two-arg show? I'm never clear what the rules are here

@asinghvi17
Copy link
Member

asinghvi17 commented Jan 13, 2025

Should we also change two-arg show

I think so, the issue with arrays of geometries vomiting all over the screen is still present :D

On another note, I just added this to GeometryOps, but still get this error:

https://github.com/JuliaGeo/GeometryOps.jl/actions/runs/12744697801/job/35517145517?pr=223#step:6:205

ERROR: LoadError: MethodError: no method matching show(::IOContext{IOBuffer}, ::MIME{Symbol("text/plain")}, ::GeoInterface.Wrappers.Point{false, false, Tuple{Float64, Float64}, Nothing}; show_mz::Bool, screen_ncols::Int64)
This error has been manually thrown, explicitly, so the method may exist but be intentionally marked as unimplemented.

Closest candidates are:
  show(::IO, ::MIME{Symbol("text/plain")}, ::GeoInterface.Wrappers.Point{Z, M, T, C}; show_mz) where {Z, M, T, C} got unsupported keyword argument "screen_ncols"
   @ GeoInterface ~/.julia/packages/GeoInterface/9xL2O/src/wrappers.jl:454
  show(::IO, ::MIME{Symbol("text/plain")}, ::Any) got unsupported keyword arguments "show_mz", "screen_ncols"
   @ Base multimedia.jl:47
  show(::IO, ::MIME{Symbol("text/plain")}, ::Core.TypeofBottom) got unsupported keyword arguments "show_mz", "screen_ncols"
   @ Base show.jl:567
  ...

Stacktrace:
  [1] kwerr(::@NamedTuple{show_mz::Bool, screen_ncols::Int64}, ::Function, ::IOContext{IOBuffer}, ::MIME{Symbol("text/plain")}, ::GeoInterface.Wrappers.Point{false, false, Tuple{Float64, Float64}, Nothing})
    @ Base ./error.jl:165
  [2] _nice_geom_str(geom::GeoInterface.Wrappers.Point{false, false, Tuple{Float64, Float64}, Nothing}, show_mz::Bool, compact::Bool, screen_ncols::Int64)
    @ GeoInterface.Wrappers ~/.julia/packages/GeoInterface/9xL2O/src/wrappers.jl:322

@rafaqz
Copy link
Member Author

rafaqz commented Jan 13, 2025

I think that's a separate bug ;)

(But I'll fix here too, and add 2 arg show)

@alex-s-gardner
Copy link
Contributor

This is a pretty nasty bug, can this PR be merged?

@rafaqz
Copy link
Member Author

rafaqz commented Jan 16, 2025

It needs tests...

@asinghvi17
Copy link
Member

asinghvi17 commented Jan 20, 2025

I added some tests, would appreciate a review before merge! One issue did come up with LineString printing in compact mode (ideally it should not show the point type) but that's a stylistic choice that we can postpone till the PR is done.

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

in all seriousness we could change the name
@rafaqz rafaqz closed this Jan 20, 2025
@alex-s-gardner alex-s-gardner mentioned this pull request Jan 20, 2025
@asinghvi17
Copy link
Member

Superseded by #185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

show bug with LibGEOS.Polygon

5 participants