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

Accept multiple EPSG codes. #22

Merged
merged 2 commits into from Jul 26, 2023
Merged

Accept multiple EPSG codes. #22

merged 2 commits into from Jul 26, 2023

Conversation

evetion
Copy link
Member

@evetion evetion commented Mar 24, 2023

It's very common to supply codes like EPSG:4326+3855 for commandline tools like gdalwarp. It means, use a horizontal crs of WGS84 (4326) and a vertical crs of EGM2008 (3855).

However, one cannot construct such a compound crs from scratch easily in JuliaGeo, as one needs to construct a valid Proj4 string with an operation like geoidgrids=grid.gtx.

This PR introduces N dimensionality to the EPSG struct, so we can do EPSG(4326, 3855) but makes sure that for EPSG{1} the behaviour is backwards compatible. Methods like ArchGDAL call val(::EPSG), and expect an Int, so using EPSG{2} will result in a MethodError.

Related to: yeesian/ArchGDAL.jl#372, which introduces a method to actually parse EPSG:4326+3855 directly. After that and this PR are merged, we can introduce another PR at ArchGDAL to use the new method with EPSG{2}.

@evetion
Copy link
Member Author

evetion commented Mar 25, 2023

@rafaqz does 👍🏻 == approval, so merge?

@rafaqz
Copy link
Member

rafaqz commented Mar 25, 2023

I prob should review, Ive just been at a conference all week and havent had time.

Thumbs up is "cool, good idea"

Copy link
Member

@rafaqz rafaqz left a comment

Choose a reason for hiding this comment

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

I'm wondering about the additional type complexity of this, and if just swapping to holding a string is better?

src/GeoFormatTypes.jl Outdated Show resolved Hide resolved
@evetion
Copy link
Member Author

evetion commented Mar 27, 2023

I'm wondering about the additional type complexity of this, and if just swapping to holding a string is better?

Well holding, constructing and parsing the string will be slower, I do really like the API of EPSG(4326).

Have you seen drawbacks for this type complexity? It seems nothing compared to our more complex ones, be it Rasters or GeoInterface wrappers. And its the same as our GeoJSON{T} or GML{X} structs?

@evetion
Copy link
Member Author

evetion commented Mar 27, 2023

I've also considered having an EPSG2 struct, or changing the field to Union{Int, NTuple{Int}}, but arrived at the current solution.

@evetion
Copy link
Member Author

evetion commented Jul 25, 2023

Bump? :)

@rafaqz
Copy link
Member

rafaqz commented Jul 26, 2023

Mostly I was worried that this is used inside Rasters and GeoInterface objects so it blows everything up a bit. But its not really by much I guess

@rafaqz rafaqz merged commit 096de54 into master Jul 26, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants