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

Getting first drop of generated EPSG database. #5

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

Conversation

fikin
Copy link

@fikin fikin commented Aug 22, 2023

It is based on epsg.properties from github.com/geotools/geotools/pull/2479.
It is based on code generation template from github.com/fikin/wkt-crs-go.

The generation currently recognizes only CRS with defined TOWGS84 entries.
And only those whose projection algorithm is supported by the lib.
All other definitions are listed at the end as comments.

It is based on epsg.properties from github.com/geotools/geotools/pull/2479.
It is based on code generation template from github.com/fikin/wkt-crs-go.
@fikin
Copy link
Author

fikin commented Aug 22, 2023

this was quite some fun to pull the generation infra up and working. but now it is ok.

there are few thinks we have to review:

  • are some of the none-TOWGS84 definitions really to be ignored or they could have some default transformation
  • are commented out projections having some easy way forward
  • and perhaps not the last, the switch case is kind of ridiculous, i'm thinking of moving towards arrays of floats for each parameter type and simply create factories out of them rather than defining functions.

how do you see it?

@wroge
Copy link
Owner

wroge commented Aug 25, 2023

That's a lot for me to look at, thanks for your effort! I will get back to you as soon as I have some more time....

@wroge
Copy link
Owner

wroge commented Oct 23, 2023

Hi, sorry, I just got around to looking at the pull request. It looks to me like you can also add the coordinate systems to the wgs84.Repository (e.g. the EPSG() function). I also wonder if the current implementation as a hash map shouldn't be more efficient than a function with a switch case.
What is your opinion?

@fikin
Copy link
Author

fikin commented Oct 23, 2023 via email

@wroge
Copy link
Owner

wroge commented Oct 23, 2023

Okay, that makes sense. Then I could remove wgs84.Repository and we would just provide a function "EPSG(code int) wgs84.CoordinateReferenceSystem".
I could clean up all the other issues with the package and provide a v2...
I think the Area Interface and Safe-functions (with errors) aren't really needed and it would be better to make the projections and spheroid public. What would you change?

@fikin
Copy link
Author

fikin commented Oct 23, 2023 via email

@wroge
Copy link
Owner

wroge commented Nov 6, 2023

It would be nice if you could contribute to this. I am also open to having the CRS generated using your tool. However, I've had a little trouble using your module or the cli...

@fikin
Copy link
Author

fikin commented Nov 12, 2023

I'll give it a try this week.
The generation process is a bit manual and not overly fond of it, I'm afraid. But it is what it is. If you feel it can be improved, pls do raise it as an issue there.

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.

2 participants