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.region: add JSON support #3941

Merged
merged 11 commits into from
Aug 11, 2024
Merged

g.region: add JSON support #3941

merged 11 commits into from
Aug 11, 2024

Conversation

kritibirda26
Copy link
Contributor

Using parson, add JSON support to g.region module.

The output looks like as follows:

{
    "projection": 99,
    "zone": 0,
    "region": {
        "north": 228500,
        "south": 215000,
        "west": 630000,
        "east": 645000,
        "ns-res": 10,
        "ns-res3": 10,
        "ew-res": 10,
        "ew-res3": 10
    },
    "top": 1,
    "bottom": 0,
    "tbres": 1,
    "rows": 1350,
    "rows3": 1350,
    "cols": 1500,
    "cols3": 1500,
    "depths": 1,
    "cells": 2025000,
    "cells3": 2025000
}

The interaction of format option with existing flags needs more looking into. We may want to disallow some combinations.

I will add tests and update documentation once the JSON format is finalized.

@github-actions github-actions bot added C Related code is in C module general labels Jun 29, 2024
@echoix echoix added this to the 8.5.0 milestone Jun 30, 2024
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.

@kritibirda26 please address the comments I left in the review and add tests to make sure the JSON format respects the various flag combinations.

general/g.region/main.c Show resolved Hide resolved
general/g.region/printwindow.c Outdated Show resolved Hide resolved
general/g.region/printwindow.c Outdated Show resolved Hide resolved
general/g.region/printwindow.c Outdated Show resolved Hide resolved
@github-actions github-actions bot added Python Related code is in Python HTML Related code is in HTML docs tests Related to Test Suite labels Jul 10, 2024
@kritibirda26
Copy link
Contributor Author

@cwhite911 I have updated the PR.

general/g.region/testsuite/test_g_region.py Outdated Show resolved Hide resolved
general/g.region/testsuite/test_g_region.py Show resolved Hide resolved
general/g.region/g.region.html Show resolved Hide resolved
@kritibirda26
Copy link
Contributor Author

@cwhite911 the pr is ready for review.

@kritibirda26 kritibirda26 requested a review from cwhite911 August 6, 2024 15:06
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.

Looks good!

Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Checked and proxying @cwhite911's approval

@echoix echoix merged commit 71f8a81 into OSGeo:main Aug 11, 2024
26 checks passed
@echoix
Copy link
Member

echoix commented Aug 12, 2024

@kritibirda26 is it possible to do a quick PR tomorrow to fix up a newly enforced ruff rule that was merged before your PR but after the last update from main of that branch?
There is one error in main, and all the following PRs to main https://github.com/OSGeo/grass/actions/runs/10341561803/job/28623503657#step:8:19

There shouldn’t be any fixes available though (for now). ruff check . --preview with pip install ruff===0.5.6

@kritibirda26
Copy link
Contributor Author

@echoix Can I disable the inspection for that block using a comment because the alternative rewrite using items would be unnecessarily convoluted?

@echoix
Copy link
Member

echoix commented Aug 12, 2024

@echoix Can I disable the inspection for that block using a comment because the alternative rewrite using items would be unnecessarily convoluted?

Doesn't this work:

        for key, value in expected.items():
            if isinstance(value, float):
                self.assertAlmostEqual(value, output_json[key], places=6)
            else:
                self.assertEqual(value, output_json[key])

@kritibirda26
Copy link
Contributor Author

I'll try that, I thought the error was because of the output_json[key] and not expected[key].

landam pushed a commit to landam/grass that referenced this pull request Aug 22, 2024
* g.region: add JSON support

* add test and documentation

* add gmt, wms to output

* fix CI build issues

* separate projection code and name fields
Mahesh1998 pushed a commit to Mahesh1998/grass that referenced this pull request Sep 19, 2024
* g.region: add JSON support

* add test and documentation

* add gmt, wms to output

* fix CI build issues

* separate projection code and name fields
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 docs general HTML Related code is in HTML libraries module Python Related code is in Python tests Related to Test Suite
Projects
Development

Successfully merging this pull request may close these issues.

3 participants