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

g.proj: add JSON support #4104

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

kritibirda26
Copy link
Contributor

@kritibirda26 kritibirda26 commented Jul 25, 2024

Use parson to add JSON support to g.proj. Currently, only works with the -p flag.

{
    "name": "Lambert Conformal Conic",
    "proj": "lcc",
    "datum": "nad83",
    "a": "6378137.0",
    "es": "0.006694380022900787",
    "lat_1": "36.16666666666666",
    "lat_2": "34.33333333333334",
    "lat_0": "33.75",
    "lon_0": "-79",
    "x_0": "609601.22",
    "y_0": "0",
    "no_defs": "defined",
    "srid": "EPSG:3358",
    "unit": "Meter",
    "units": "Meters",
    "meters": "1"
}

I tried to add support for some other flags as well but it seems the SHELL/PLAIN output needs to be parsed manually to obtain the data and there is no direct way to obtain the data directly, for instance for w flag.

Also, some floats are represented as strings at the moment and need to be fixed.

@github-actions github-actions bot added C Related code is in C module general labels Jul 25, 2024
@cwhite911
Copy link
Contributor

For the proj4 format flag -j and the WKT format flag -w you can just return the the flattened (flag -f) string to the json.

{
   "wtk": "string_value",
   "proj4": "string_value"
}

Use parson to add JSON support to g.proj. Currently, only works with the -p
flag.
@kritibirda26
Copy link
Contributor Author

@cwhite911 I have made the requested changes.

Copy link
Contributor

@cwhite911 cwhite911 left a comment

Choose a reason for hiding this comment

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

Looking good. Please add tests and update the docs.

general/g.proj/output.c Show resolved Hide resolved
general/g.proj/output.c Show resolved Hide resolved
@cwhite911
Copy link
Contributor

From @florisvdh on issue #3020

Adding here that the preferred JSON encoding for g.proj output would be the PROJJSON specification: https://proj.org/en/latest/specifications/projjson.html. It's a JSON encoding of WKT2:2019.

@kritibirda26
Copy link
Contributor Author

@cwhite911 I tried to update the output to use projjson specification, however there seem to be a lot of cases to handle. IIUC, there are 3 different major versions (4/5/6) of proj4 that need to be handled separately and another case of not having libproj4 but using internal Grass GIS functions. FWIW, libproj4 starting with v6 exposes a function to generate the projjson representation directly.

While reading the code in get_proj.c and related files, I saw some comments about a future cleanup of proj4 versions. I think it would be better to handle add the proper JSON representation of proj4 here at that stage.

@cwhite911
Copy link
Contributor

@kritibirda26

@cwhite911 I tried to update the output to use projjson specification, however there seem to be a lot of cases to handle. IIUC, there are 3 different major versions (4/5/6) of proj4 that need to be handled separately and another case of not having libproj4 but using internal Grass GIS functions. FWIW, libproj4 starting with v6 exposes a function to generate the projjson representation directly.

While reading the code in get_proj.c and related files, I saw some comments about a future cleanup of proj4 versions. I think it would be better to handle add the proper JSON representation of proj4 here at that stage.

@kritibirda26 that sounds good to me. Thanks for looking into it!

@echoix
Copy link
Member

echoix commented Aug 26, 2024

Are we even running build that use proj4? I only remember seeing proj 5+ api at the end of configure, and now Proj is at 9.4 now.

@neteler
Copy link
Member

neteler commented Aug 27, 2024

Are we even running build that use proj4?

According to configure.ac we support "PROJ.4.4.6 or later".

@echoix
Copy link
Member

echoix commented Nov 7, 2024

Do we want to go forward finishing up and merging the existing json PRs kinda soon? since some work is being done to implement on other modules too.

Copy link
Contributor

@cwhite911 cwhite911 left a comment

Choose a reason for hiding this comment

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

We should merge this as it addresses the main purpose of the PR and make a new issues to discuss incorporating the PROJJSON specification.

@cwhite911
Copy link
Contributor

@echoix same issue here. I am no longer able to resolve conversations.

@echoix
Copy link
Member

echoix commented Jan 23, 2025

Ok, resolved them. Do you think the tests failures are transient and I should update the PR, or there's some real changes to do?

@echoix
Copy link
Member

echoix commented Jan 23, 2025

Because of the open comments, when doing my rounds of PRs to merge, I didn't act on it.

@cwhite911 cwhite911 self-assigned this Jan 23, 2025
Copy link
Contributor

@cwhite911 cwhite911 left a comment

Choose a reason for hiding this comment

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

We need to fix the shell format output when the g flag is set.

for (i = 0; i < projinfo->nitems; i++) {
if (strcmp(projinfo->key[i], "init") == 0)
continue;
if (shell)
switch (format) {
case PLAIN:
fprintf(stdout, "%s=%s\n", projinfo->key[i], projinfo->value[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fprintf(stdout, "%s=%s\n", projinfo->key[i], projinfo->value[i]);
fprintf(stdout, "%-11s: %s\n", projinfo->key[i],
projinfo->value[i]);

Comment on lines 64 to 65
fprintf(stdout, "%-11s: %s\n", projinfo->key[i],
projinfo->value[i]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fprintf(stdout, "%-11s: %s\n", projinfo->key[i],
projinfo->value[i]);
fprintf(stdout, "%s=%s\n", projinfo->key[i], projinfo->value[i]);

@cwhite911
Copy link
Contributor

@kritibirda26 will you please accept these changes so we can merge this PR.

@cwhite911
Copy link
Contributor

Ok, resolved them. Do you think the tests failures are transient and I should update the PR, or there's some real changes to do?

Real changes. Nice catch @echoix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C general module
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants