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.describe: Add JSON support #4918

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

Conversation

NishantBansal2003
Copy link
Contributor

@NishantBansal2003 NishantBansal2003 commented Jan 5, 2025

fixes: #4903
Use parson to add json output format support to the r.describe module.
The JSON output looks like as follows:

{
            "null_value": "*",
            "ranges": [
                {"min": -5, "max": -4.96078431372549},
                {"min": -4.529411764705882, "max": -4.490196078431373},
                {"min": -4.019607843137255, "max": -3.980392156862745},
                {"min": -3.5098039215686274, "max": -3.4705882352941178},
                {"min": -3.0392156862745097, "max": -3},
                {"min": -2.5294117647058822, "max": -2.4901960784313726},
                {"min": -2.019607843137255, "max": -1.9803921568627452},
                {"min": -1.549019607843137, "max": -1.5098039215686274},
                {"min": -1.0392156862745097, "max": -1},
                {"min": -0.5294117647058822, "max": -0.4901960784313726},
                {"min": 0.4509803921568629, "max": 0.4901960784313726},
                {"min": 0.9607843137254903, "max": 1},
                {"min": 1.4705882352941178, "max": 1.5098039215686274},
                {"min": 1.9411764705882355, "max": 1.9803921568627452},
                {"min": 2.450980392156863, "max": 2.4901960784313726},
                {"min": 2.9607843137254903, "max": 3},
                {"min": 3.431372549019608, "max": 3.4705882352941178},
                {"min": 3.9411764705882355, "max": 3.980392156862745},
                {"min": 4.450980392156863, "max": 4.490196078431373},
                {"min": 4.96078431372549, "max": 5},
            ],
}

Signed-off-by: Nishant Bansal <[email protected]>
Signed-off-by: Nishant Bansal <[email protected]>
@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 5, 2025
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
raster/r.describe/testsuite/test_r_describe.py Outdated Show resolved Hide resolved
Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003
Copy link
Contributor Author

Hi everyone, could someone please review this PR when you have a moment? Thank you!

@echoix
Copy link
Member

echoix commented Jan 10, 2025

About the format shown in the body of your PR: Does this mean that the root of the json output is an array instead of an object (like {})? I didn't know it was valid json. I see that the similat shape is used in the test.

For your PR, I don't think I'm able to judge about the C part. But for the Python and test part I could check out.

@NishantBansal2003
Copy link
Contributor Author

About the format shown in the body of your PR: Does this mean that the root of the json output is an array instead of an object (like {})? I didn't know it was valid json. I see that the similat shape is used in the test.

For your PR, I don't think I'm able to judge about the C part. But for the Python and test part I could check out.

Yes, the format will be an array of objects, each containing a category value (originally separated by newlines in the plain format). However, if this format is not appropriate, we can discuss and consider a new output format.
Sure, please take a look at the Python and test parts.
Thanks!

Signed-off-by: Nishant Bansal <[email protected]>
Signed-off-by: Nishant Bansal <[email protected]>
Signed-off-by: Nishant Bansal <[email protected]>
@NishantBansal2003
Copy link
Contributor Author

Hey @petrasovaa, I tried following the suggestion from #4903 (comment) with some minor adjustments. Could you take a look?

@petrasovaa
Copy link
Contributor

Hey @petrasovaa, I tried following the suggestion from #4903 (comment) with some minor adjustments. Could you take a look?

That was fast! This looks great, I will test it more tomorrow and let you know if anything else is needed.

@petrasovaa
Copy link
Contributor

I finally tested it and the only thing I would change is the null reporting, it seems to me that a boolean like "has_nulls" makes more sense here for the JSON, so I would ignore the nv parameter.

The test would be normally better done as a separate PR before r.describe is modified, but I tested the plain format parts of the test without the changes so it's fine as it is now.

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
Status: No status
Development

Successfully merging this pull request may close these issues.

[Feat] Add JSON output to r.describe
3 participants