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.

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

Implement CRSmode so we can dispatch on the type of crs. #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Implement CRSmode so we can dispatch on the type of crs. #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions src/GeoFormatTypes.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ contains geometry data.
struct Geom <: FormatMode end
Base.show(io::IO, ::MIME"text/plain", ::Type{Geom}) = print(io, "Geometry mode")

"""
CRSMode

Traits to indicate the CRS type, such as `ProjectedCRS`, `GeographicCRS` or `UnknownCRS`.
"""
abstract type CRSMode end
struct ProjectedCRS <: CRSMode end
struct GeographicCRS <: CRSMode end
struct UnknownCRS <: CRSMode end

"""
CRS <: FormatMode

Expand All @@ -47,8 +57,9 @@ Base.show(io::IO, ::MIME"text/plain", ::Type{Geom}) = print(io, "Geometry mode")
Trait specifying that a format object, like [`WellKnownText`](@ref),
contains only coordinate reference system data.
"""
struct CRS <: FormatMode end
Base.show(io::IO, ::MIME"text/plain", ::Type{CRS}) = print(io, "CRS mode")
struct CRS{T<:CRSMode} <: FormatMode end
CRS() = CRS{UnknownCRS}()
Base.show(io::IO, ::MIME"text/plain", ::Type{CRS{T}}) where {T} = print(io, "$T mode")
Comment on lines -50 to +62
Copy link
Member

Choose a reason for hiding this comment

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

AFAICT, this won't work for the CoordinateReferenceSystemFormat types?

Maybe we need either a wrapper as indicated in the issue, or an extra type param in the type definition for those

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I guess they will all need a type parameter.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm @asinghvi17 your comment in the issue makes a good point that wrapping the other way is simpler, instead of adding heaps of type parameters to every object here we just compose the objects here as-is with other singletons.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you show a MWE for a wrapper? I'd like to adjust the existing types to use them as they're implemented across the ecosystem.

Copy link
Member

@asinghvi17 asinghvi17 Oct 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I was thinking of something on the lines of:

struct CRS{T, CRSTrait} <: CoordinateReferenceSystemFormat
	val::T 
	CRS(x::CoordinateReferenceSystemFormat, ::CRSTrait) where CRSTrait = CRS{typeof(x), CRSTrait}(x)
end

CRS(x::CoordinateReferenceSystemFormat) = CRS(x, GI.crstrait(x))

Copy link
Member

Choose a reason for hiding this comment

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

then, the GI.crs of a geometry might be CRS{EPSG, GeographicCRSTrait}(EPSG((4326,)))

Copy link
Member

Choose a reason for hiding this comment

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

That MWE probably won't work as is, but you get the idea


"""
MixedFormatMode <: FormatMode
Expand Down
10 changes: 5 additions & 5 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
using GeoFormatTypes, Test
using GeoFormatTypes: Geom, CRS, Extended, Unknown
using GeoFormatTypes: Geom, CRS, Extended, Unknown, UnknownCRS

@testset "Test construcors" begin
@test_throws ArgumentError ProjString("+lat_ts=56.5 +ellps=GRS80")
Expand All @@ -23,11 +23,11 @@ end
@test ESRIWellKnownText("test") isa ESRIWellKnownText{Unknown}
@test WellKnownText(Extended(), "test") isa WellKnownText{Extended}
@test WellKnownBinary(Extended(), [1, 2, 3, 4]) isa WellKnownBinary{Extended}
@test WellKnownText2(CRS(), "test") isa WellKnownText2{CRS}
@test WellKnownText2(CRS(), "test") isa WellKnownText2{CRS{UnknownCRS}}
@test ESRIWellKnownText(Geom(), "test") isa ESRIWellKnownText{Geom}
@test GML("test") isa GML{Unknown}
@test GML(Geom(), "test") isa GML{Geom}
@test GML(CRS(), "test") isa GML{CRS} # Probably doesn't actually exist
@test GML(CRS(), "test") isa GML{CRS{UnknownCRS}} # Probably doesn't actually exist
@test KML("test") isa KML
@test GeoJSON("test") isa GeoJSON
end
Expand Down Expand Up @@ -125,11 +125,11 @@ Base.convert(target::Type{<:GeoFormat}, mode::Union{CRS,Type{CRS}}, source::GeoF
(ESRIWellKnownText("test"), ("ESRIWellKnownText", "ESRIWellKnownText with Unknown mode: test")),
(WellKnownText(Extended(), "test"), ("WellKnownText", "WellKnownText with Extended mode: test")),
(WellKnownBinary(Extended(), [1, 2, 3, 4]), ("WellKnownBinary", "WellKnownBinary with Extended mode: [1, 2, 3, 4]")),
(WellKnownText2(CRS(), "test"), ("WellKnownText2", "WellKnownText2 with CRS mode: test")),
(WellKnownText2(CRS(), "test"), ("WellKnownText2", "WellKnownText2 with UnknownCRS mode: test")),
(ESRIWellKnownText(Geom(), "test"), ("ESRIWellKnownText", "ESRIWellKnownText with Geometry mode: test")),
(GML("test"), ("GML", "GML with Unknown mode: test")),
(GML(Geom(), "test"), ("GML", "GML with Geometry mode: test")),
(GML(CRS(), "test"), ("GML", "GML with CRS mode: test")),
(GML(CRS(), "test"), ("GML", "GML with UnknownCRS mode: test")),
(KML("test"), ("KML", "KML: test")),
(GeoJSON("test"), ("GeoJSON String", "GeoJSON String: test")),
]
Expand Down
Loading