CoordRefSystems.jl icon indicating copy to clipboard operation
CoordRefSystems.jl copied to clipboard

add EPSG:31370

Open tcarion opened this issue 6 months ago • 4 comments

reference: https://epsg.org/crs_31370/BD72-Belgian-Lambert-72.html

tcarion avatar Apr 30 '25 11:04 tcarion

the conversion method use Lambert Conic Conformal, but I couldn't find reference to it in the code. Is it currently missing?

tcarion avatar Apr 30 '25 11:04 tcarion

Thank you for the PR @tcarion !

the conversion method use Lambert Conic Conformal, but I couldn't find reference to it in the code. Is it currently missing?

That is right. We need to implement the formula first, in order to attach the correct CRS type to the EPSG code instead of the generic Cartesian2D.

The formula is well-documented in the link shared, but I would make sure they match the formulas in the official EPSG guide. Do you think you have some time to work on this new LambertConic type?

juliohm avatar Apr 30 '25 12:04 juliohm

Sure, I can try to help!

Do you think I should do this here or rather create another PR?

tcarion avatar Apr 30 '25 12:04 tcarion

Another PR would be great. That way we can focus the review on the formulas and behavior. You could leave this PR open and rebase later after the formula is available.

Em qua., 30 de abr. de 2025, 09:56, Tristan Carion @.***> escreveu:

tcarion left a comment (JuliaEarth/CoordRefSystems.jl#263) https://github.com/JuliaEarth/CoordRefSystems.jl/pull/263#issuecomment-2841886971

Sure, I can try to help!

Do you think I should do this here or rather create another PR?

— Reply to this email directly, view it on GitHub https://github.com/JuliaEarth/CoordRefSystems.jl/pull/263#issuecomment-2841886971, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZQW3NJNZTE3O76HWQTK6T24DB7DAVCNFSM6AAAAAB4FNNMLCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNBRHA4DMOJXGE . You are receiving this because you commented.Message ID: @.***>

juliohm avatar Apr 30 '25 13:04 juliohm

@tcarion I believe we can map the new CRS type to the EPSG code now 🙂

juliohm avatar Jun 16 '25 10:06 juliohm

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 91.85%. Comparing base (395b9c1) to head (c81d172). Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/datums.jl 0.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #263      +/-   ##
==========================================
- Coverage   91.90%   91.85%   -0.06%     
==========================================
  Files          40       40              
  Lines        1754     1755       +1     
==========================================
  Hits         1612     1612              
- Misses        142      143       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov-commenter avatar Jun 16 '25 11:06 codecov-commenter

For the parameters, I changed them to be the same as the projstring given by proj:

$ projinfo EPSG:31370
PROJ.4 string:
+proj=lcc +lat_0=90 +lon_0=4.36748666666667 +lat_1=51.1666672333333 +lat_2=49.8333339 +x_0=150000.013 +y_0=5400088.438 +ellps=intl +units=m +no_defs +type=crs

Do you think I should precise this in the comments?

Also, I would like to add the transformation from BD72 to WGS84 in transforms.jl. I used the parameters from this page, but it gives slightly different results than Proj, which makes the test fail

tcarion avatar Jun 16 '25 14:06 tcarion

Do you think I should precise this in the comments?

Perhaps a short comment explaining the choice can help future contributors. We can explain that the EPSG database shows parameters in DMS, and because of that we opted for the C PROJ parameters.

Also, I would like to add the transformation from BD72 to WGS84 in transforms.jl. I used the parameters from this page, but it gives slightly different results than Proj, which makes the test fail

That could be done in a separate PR for better review.

juliohm avatar Jun 16 '25 14:06 juliohm