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

r.what: Add JSON output #3528

Merged
merged 5 commits into from
Mar 28, 2024
Merged

r.what: Add JSON output #3528

merged 5 commits into from
Mar 28, 2024

Conversation

kritibirda26
Copy link
Contributor

Added an option format which accepts the value "plain" for the current output format and "json" for the JSON output. The newly added parson library is being used here.

Also, added a couple of python tests to ensure JSON output works as expected.

fixes #3518

Added an option format which accepts the value "plain" for the current
output format and "json" for the JSON output. The newly added parson library
is being used here.

Also, added a couple of python tests to ensure JSON output works as expected.
@github-actions github-actions bot added raster Related to raster data processing Python Related code is in Python C Related code is in C module tests Related to Test Suite labels Mar 24, 2024
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
@echoix
Copy link
Member

echoix commented Mar 24, 2024

I think you can ignore (resolve) the Clang-Format suggestions that are already marked as outdated (since you fixed them before they got published, they were queued from other CI activity in the OSGeo organization)

@kritibirda26
Copy link
Contributor Author

Thanks for the tip, @echoix.

@echoix
Copy link
Member

echoix commented Mar 24, 2024

Is the way the tests written testing the json output, or python's json parsing?

@kritibirda26
Copy link
Contributor Author

kritibirda26 commented Mar 24, 2024

The JSON is output by the r.what tool and then it loaded by using python's json module. Python's json module loading the output of the r.what tool successfully shows that the output was valid JSON. Then the loaded output is compared with the reference python object. So as far as I understand and intend, the tests are testing the right thing.

If you prefer, I can hardcode the json outputs and avoid using json.loads.

@ninsbl
Copy link
Member

ninsbl commented Mar 24, 2024

Nice. I wonder if it would make sense to create a standard parser option (e.g. G_OPT_M_FORMAT) for string output formats? Not needed for this PR, but could be helpful for wider implementation of JSON output across various modules...

@echoix
Copy link
Member

echoix commented Mar 24, 2024

Nice. I wonder if it would make sense to create a standard parser option (e.g. G_OPT_M_FORMAT) for string output formats? Not needed for this PR, but could be helpful for wider implementation of JSON output across various modules...

Totally agree that adding json output to many modules should be using a standard option, it will be duplicated many times otherwise.

@kritibirda26
Copy link
Contributor Author

If someone can open an issue with more details on what to do, I'll be happy to implement it.

@echoix
Copy link
Member

echoix commented Mar 24, 2024

If someone can open an issue with more details on what to do, I'll be happy to implement it.

It's not quite finished discussing: #3410

@echoix echoix added this to the 8.4.0 milestone Mar 24, 2024
@petrasovaa
Copy link
Contributor

petrasovaa commented Mar 25, 2024

This looks great @kritibirda26, but now I wonder if we could try a different layout for the JSON. Currently the layout is similar to the csv output, but I was thinking something like:

{
  "data": [
    {
      "easting": 123.456,
      "northing": 78.901,
      "elevation": {"value": 345.67, "color": "#ff0000"},
      "slope": {"value": 12.3, "color": "#00ff00"},
      "aspect": {"value": 45.6, "color": "#0000ff"}
    },
    {
      "easting": 234.567,
      "northing": 89.012,
      "elevation": {"value": 456.78, "color": "#ff0000"},
      "slope": {"value": 23.4, "color": "#00ff00"},
      "aspect": {"value": 56.7, "color": "#0000ff"}
    }
  ]
}

or even simpler

[
    {
      "easting": 123.456,
      "northing": 78.901,
      "elevation": {"value": 345.67, "color": "#ff0000"},
      "slope": {"value": 12.3, "color": "#00ff00"},
      "aspect": {"value": 45.6, "color": "#0000ff"}
    },
    {
      "easting": 234.567,
      "northing": 89.012,
      "elevation": {"value": 456.78, "color": "#ff0000"},
      "slope": {"value": 23.4, "color": "#00ff00"},
      "aspect": {"value": 56.7, "color": "#0000ff"}
    }
]

Would it be sufficiently easy to redo it this way?

@petrasovaa
Copy link
Contributor

If someone can open an issue with more details on what to do, I'll be happy to implement it.

Let's leave this for later.

At this point it would be good to draft a proposal based on the template specified here and introduce yourself on the grass-dev mailing list. See the tips on the idea page as well. Share the proposal in a google doc or similar so that we can comment. Let us know if you have questions!

raster/r.what/main.c Outdated Show resolved Hide resolved
@cwhite911
Copy link
Contributor

This looks great @kritibirda26, but now I wonder if we could try a different layout for the JSON. Currently the layout is similar to the csv output, but I was thinking something like:

{
  "data": [
    {
      "easting": 123.456,
      "northing": 78.901,
      "elevation": {"value": 345.67, "color": "#ff0000"},
      "slope": {"value": 12.3, "color": "#00ff00"},
      "aspect": {"value": 45.6, "color": "#0000ff"}
    },
    {
      "easting": 234.567,
      "northing": 89.012,
      "elevation": {"value": 456.78, "color": "#ff0000"},
      "slope": {"value": 23.4, "color": "#00ff00"},
      "aspect": {"value": 56.7, "color": "#0000ff"}
    }
  ]
}

or even simpler

[
    {
      "easting": 123.456,
      "northing": 78.901,
      "elevation": {"value": 345.67, "color": "#ff0000"},
      "slope": {"value": 12.3, "color": "#00ff00"},
      "aspect": {"value": 45.6, "color": "#0000ff"}
    },
    {
      "easting": 234.567,
      "northing": 89.012,
      "elevation": {"value": 456.78, "color": "#ff0000"},
      "slope": {"value": 23.4, "color": "#00ff00"},
      "aspect": {"value": 56.7, "color": "#0000ff"}
    }
]

Would it be sufficiently easy to redo it this way?

It might be worth returning the data as geojson.

{
  "type": "FeatureCollection",
  "bbox": [100.0, 0.0, 105.0, 1.0],
  "features": [
    {
      "type": "Feature",
      "geometry": {
        "type": "Point",
        "coordinates": [123.456, 78.901]
      },
      "properties": {
        "elevation": {
          "value": 345.67,
          "color": "#ff0000"
        },
        "slope": {
          "value": 12.3,
          "color": "#00ff00"
        },
        "aspect": {
          "value": 45.6,
          "color": "#0000ff"
        }
      }
    },
    {
      "type": "Feature",
      "geometry": {
        "type": "Point",
        "coordinates": [234.567, 89.012]
      },
      "properties": {
        "elevation": {
          "value": 456.78,
          "color": "#ff0000"
        },
        "slope": {
          "value": 23.4,
          "color": "#00ff00"
        },
        "aspect": {
          "value": 56.7,
          "color": "#0000ff"
        }
      }
    }
  ]
}

@petrasovaa
Copy link
Contributor

It might be worth returning the data as geojson.

Good point, but perhaps as an additional format? I think having a simple json covers most cases, you just want the value.

@kritibirda26
Copy link
Contributor Author

This looks great @kritibirda26, but now I wonder if we could try a different layout for the JSON. Currently the layout is similar to the csv output, but I was thinking something like:
@petrasovaa, The actual json output doesn't have any elevation or aspect data so to confirm this is about changing items in the array from

{
    "easting": "332533.5941495",
    "northing": "242831.139883875",
    "site_name": "",
    "boundary_county_500m": 121,
    "boundary_county_500m_color": "010:000:255",
}

to

{
    "easting": "332533.5941495",
    "northing": "242831.139883875",
    "site_name": "",
    "boundary_county_500m": { "value": 121, "color": "010:000:255" }
}

@petrasovaa
Copy link
Contributor

@petrasovaa, The actual json output doesn't have any elevation or aspect data so to confirm this is about changing items in the array from

Yes, elevation and aspect were just examples of raster layers.

@kritibirda26
Copy link
Contributor Author

Hi! @cwhite911 and @petrasovaa, I have applied the suggestions and also updated the json format as suggested.

raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
raster/r.what/main.c Outdated Show resolved Hide resolved
@petrasovaa
Copy link
Contributor

@kritibirda26 since this is getting close to being done, would you like to start working on the GSoC proposal? Deadline is next week.

@kritibirda26
Copy link
Contributor Author

@petrasovaa I have started working on the GSoC proposal. I aim to share a draft by tomorrow on the mailing list. I have one question about it. For a 350 hours project, how many tools should I aim to add JSON support for?

@petrasovaa
Copy link
Contributor

@petrasovaa I have started working on the GSoC proposal. I aim to share a draft by tomorrow on the mailing list. I have one question about it. For a 350 hours project, how many tools should I aim to add JSON support for?

Good question, I will have to think about it, some tools will be more work than others. You can try to guess also based on how much time you spent on this PR. And it's more about the quality. Start the proposal and I and others will comment on it and prioritize the tools.

@cwhite911
Copy link
Contributor

cwhite911 commented Mar 27, 2024

@petrasovaa second option (the one that is implemented here) integrates really well with pandas dataframes.

import pandas as pd

grass_out = gs.read_command("r.what", map="elevation,slope,aspect", points=samples, format="json", flags="r")
# example output
grass_out = [
    {
      "easting": 123.456,
      "northing": 78.901,
      "elevation": {"value": 345.67, "color": "#ff0000"},
      "slope": {"value": 12.3, "color": "#00ff00"},
      "aspect": {"value": 45.6, "color": "#0000ff"}
    },
    {
      "easting": 234.567,
      "northing": 89.012,
      "elevation": {"value": 456.78, "color": "#ff0000"},
      "slope": {"value": 23.4, "color": "#00ff00"},
      "aspect": {"value": 56.7, "color": "#0000ff"}
    }
]

df_n = pd.json_normalize(json.loads(grass_out), max_level=1)
df_n.head()

Outputs

easting northing elevation.value elevation.color slope.value slope.color aspect.value aspect.color
0 123.456 78.901 345.67 #ff0000 12.3 #00ff00 45.6 #0000ff
1 234.567 89.012 456.78 #ff0000 23.4 #00ff00 56.7 #0000ff

Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

This looks ready to go!

@echoix echoix changed the title [feat] Add JSON output to r.what r.what: Add JSON output Mar 28, 2024
@echoix echoix merged commit fbc99e8 into OSGeo:main Mar 28, 2024
26 checks passed
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 module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] Add JSON output to r.what
6 participants