-
Notifications
You must be signed in to change notification settings - Fork 11
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
WIP: Add EPSG 3031 #240
base: main
Are you sure you want to change the base?
WIP: Add EPSG 3031 #240
Conversation
That is a very good start @ThatcherC ! 👏🏽🙂
That is probably due to the type constructor with/without datum. Try to match the code pattern of other CRS types before moving forward.
It would be super welcome to nail them down now that you are working on it. Check the struct PolarStereographic{Variant,Hemisphere,lat₁,Datum,Shift,M<:Met} <: Projected{Datum,Shift}
x::M
y::M
end
const PolarStereographicNorthA = PolarStereographic{:A,:North}
const PolarStereographicNorthB = PolarStereographic{:A,:North}
Make sure to add a test against PROJ as we do with other CRS types here
They are the formulas for the projected coordinates, without units and without scaling (multiplication by major axis). We have additional material in GDSJL Chapter 6: https://juliaearth.github.io/geospatial-data-science-with-julia/06-projections.html |
Thanks @juliohm! I've pushed a few updates. Removing the datum from the type constructor definitely helped the tests. I'm now seeing issues with the approximate equality tests and the conversion tests against Proj (both of which I think might be a type stability issue). I'll take a look at the A and C variants too - that reference to the |
@@ -120,6 +120,7 @@ export | |||
Sinusoidal, | |||
LambertAzimuthalEqualArea, | |||
EqualEarth, | |||
PolarStereographicB, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we rename it to PolarStereographic
already? I understand B is just one subtype.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! I'll make types for the A, B, and C variants. Should they be subtypes of Mode
like EllipticalMode
in src/modes.jl
? I could also make an abstract type Variant
with subtypes A, B, and C (either in src/modes.jl
or in a new file like ``src/variants.jl` with a similar layout).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice if you could encapsulate these variants in the same source file where you are defining PolarStereographic, assuming these variants are specific to this CRS.
If you think that these variants are general enough to be used by other CRS types, then we can consider a shared file like src/modes.jl or src/variants.jl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good - I'll keep them in the polarstereographic
file for now. Easy to move them later if need be!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, Variant
types have been added. I've updated the code so that the PolarStereographic type with the B Variant works, the PolarStereographicB type works, and the Polar Stereographic type with the A and C variants do not work because they are unimplemented (for now):
julia> convert(LatLon, CoordRefSystems.PolarStereographic{CoordRefSystems.B, -71°}(10,10))
GeodeticLatLon{WGS84Latest} coordinates
├─ lat: -89.99986984060983°
└─ lon: 45.0°
julia> convert(LatLon, PolarStereographicB{-71°}(10,10))
GeodeticLatLon{WGS84Latest} coordinates
├─ lat: -89.99986984060983°
└─ lon: 45.0°
julia> convert(LatLon, CoordRefSystems.PolarStereographic{CoordRefSystems.A, -71°}(10,10))
ERROR: MethodError: no method matching formulas(::Type{CoordRefSystems.PolarStereographic{…}}, ::Type{Float64})
The function `formulas` exists, but no method is defined for this combination of argument types.
...
I'm not making use of the North
and South
modes because according the EPSG guidance notes that "In the
formulas that follow for these variants B and C, the hemisphere of pole is taken to be that of the hemisphere
of the standard parallel." It'll be a little different for Variant A, but I'd prefer to keep this PR focused on PolarStereographicB and EPSG:3031 for now.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #240 +/- ##
==========================================
+ Coverage 93.12% 93.33% +0.21%
==========================================
Files 34 35 +1
Lines 1643 1695 +52
==========================================
+ Hits 1530 1582 +52
Misses 113 113 ☔ View full report in Codecov by Sentry. |
Alternatively, the Polar Stereographic Projection can be implemented generically using the Stereographic Projection, as PROJ does. This is the PROJ string for the EPSG:3031: Note that PROJ defines South Pole Stereographic Projection by setting Formula reference: https://neacsu.net/docs/geodesy/snyder/5-azimuthal/sect_21/#formulas-for-the-ellipsoid |
I tend to favor these general approaches, and agree with @eliascarv that it would be great if we could introduce a general Do you think it would be possible to refactor the PR to accommodate this more general design, but only implement the part that you care about for now? That way we avoid introducing a In the meantime, I would like to thank you all for the great research and work that has been done here. It is amazing to see progress and to learn about new projections as we further develop the package. |
Addresses issue #239! I have a minimal working implement of the EPSG 3031 CRS. It produces the same forward and backward values as the example given in EPSG guidance note #7-2, although it is currently failing some of the Pkg test due to a type error somewhere.
Here's the EPSG:3031 to WGS84 geodetic lat/lon conversion working forward and backwards, and getting the same values as Proj.jl:
Right now I've only implement the Polar Stereographic Variant B projection (there's also A and C variants) and I've ignored the north pole case. Is the north pole case and the A and B variants something that'd be valuable to have implemented as well? Currently I don't have a need for them but they'll look a lot like the Polar Stereographic B case so I could try implementing them if there's interest.
I'd love any advice on where to go next with this! Especially if anyone has insight into how to get the failing tests to pass, and whether I should add any more EPSG:3031-specific test.
On another topic, are there docs on what
fx
andfy
are supposed to be? That'd be nice info to add into a guide for contribution. I found these functions a little confusing, particularly the part where they seem to be set up to not input and output dimension quantities.