Skip to content

r.geomorphon: use gjson/parson for JSON profile output#6984

Open
Abhi-d-gr8 wants to merge 4 commits intoOSGeo:mainfrom
Abhi-d-gr8:feat/6970-rgeomorphon-parson
Open

r.geomorphon: use gjson/parson for JSON profile output#6984
Abhi-d-gr8 wants to merge 4 commits intoOSGeo:mainfrom
Abhi-d-gr8:feat/6970-rgeomorphon-parson

Conversation

@Abhi-d-gr8
Copy link
Contributor

Fixes #6970

This updates r.geomorphon JSON profile output to use the GRASS JSON wrapper (gjson, backed by parson) instead of manual string formatting.

Changes :

  • profile.c: build the JSON output via gjson (G_json_* API) and serialize pretty

  • Makefile: link against $(PARSONLIB) and add $(PARSONDEP)

  • testsuite/test_r_geom.py: add test_profile_json to confirm the output file is created and contains expected top-level keys

  • YAML/XML output paths are unchanged (per guidance to keep the PR focused on JSON).

  • Goal is a pure refactor of JSON generation with the same external behavior/structure.

AI usage note :
I used AI assistance while working on this because I was hitting repeated syntax/build issues and initially couldn’t locate the GRASS JSON wrapper. It helped point me to gjson and the general direction, but I implemented the final changes and validated with local build + pre-commit.

@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 Jan 28, 2026
@pahalsrivastava
Copy link
Contributor

Hi, was looking at the CI results, the failures seem to be isolated to certain builds, it looks more like a CMake or parson/gjson linkage or avaliability issue in the minimal builds, rather than a problem with the JSON generation logic itself. I am familiar with the r.kappa json path and the cmake setup, would be glad to help or debug if that is useful.

@Abhi-d-gr8 Abhi-d-gr8 force-pushed the feat/6970-rgeomorphon-parson branch from 4ff2e5c to 845e85a Compare January 28, 2026 23:59
@Abhi-d-gr8
Copy link
Contributor Author

Hi ! @pahalsrivastava
Yes please go ahead. I think it’s a CMake/minimal-build gjson/parson linkage issue. Any pointers/fix would be really helpful, thanks!

@github-actions github-actions bot added the CMake label Feb 3, 2026
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

The test will need to go in first in a separate PR. Then this PR should follow it. Get a good test in, then we can back to this PR.

Comment on lines +96 to +97
fd, tmp = tempfile.mkstemp(prefix="rgeom_profile_", suffix=".json")
os.close(fd)
Copy link
Member

Choose a reason for hiding this comment

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

Why are you creating it here?

Comment on lines +112 to +116
# basic sanity checks on JSON structure
self.assertIn("final_results", data)
self.assertIn("computation_parameters", data)
self.assertIsInstance(data["final_results"], dict)
self.assertIsInstance(data["computation_parameters"], dict)
Copy link
Member

Choose a reason for hiding this comment

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

Actual (current) values will need to be checked.

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 CMake 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] Rewrite r.geomorphon JSON output to use parson library

3 participants