Skip to content

Comments

r.horizon: add json output#3534

Merged
petrasovaa merged 3 commits intoOSGeo:mainfrom
petrasovaa:r.horizon-json
Mar 26, 2024
Merged

r.horizon: add json output#3534
petrasovaa merged 3 commits intoOSGeo:mainfrom
petrasovaa:r.horizon-json

Conversation

@petrasovaa
Copy link
Contributor

@petrasovaa petrasovaa commented Mar 25, 2024

This PR adds JSON format to r.horizon. Adds option format which can be plain (default, the previous output format) or json.

r.horizon elevation=elevation@PERMANENT direction=50 step=50 coordinates=632431.8242769501,222607.44281332166 format=json

[
    {
        "x": 632431.824277,
        "y": 222607.442813,
        "azimuth": [
            50.000000,
            100.000000,
            150.000000,
            200.000000,
            250.000000,
            300.000000,
            350.000000
        ],
        "horizon_height": [
            0.031715,
            0.065790,
            0.084245,
            0.084245,
            0.008515,
            0.000000,
            0.000000
        ]
    }
]

I am open to alternative JSON schemes if others think they would be better. For the context, I plan to add support for multiple points.

@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 25, 2024
@petrasovaa petrasovaa self-assigned this Mar 25, 2024
nilason
nilason previously approved these changes Mar 26, 2024
View reviewed changes
Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

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

Over all this looks fine to me. I have a couple of suggestions, which you may certainly dismiss if you like as they are only style related. Your, in my opinion, good choice of using enum for type of format, begs to use switch statement instead of if..else. In addition to a clearer, albeit a bit more wordy, code, it has the advantage of being checked by the compiler, should the format options change.

@petrasovaa
Copy link
Contributor Author

petrasovaa commented Mar 26, 2024

Over all this looks fine to me. I have a couple of suggestions, which you may certainly dismiss if you like as they are only style related. Your, in my opinion, good choice of using enum for type of format, begs to use switch statement instead of if..else. In addition to a clearer, albeit a bit more wordy, code, it has the advantage of being checked by the compiler, should the format options change.

I was mostly stealing from v.db.select implementation:) I can change it to switch, it's just I forgot it even existed since it was not in Python (looks like it is there now).

@nilason nilason mentioned this pull request Mar 26, 2024
Merged
@neteler neteler added this to the 8.4.0 milestone Mar 26, 2024
@petrasovaa petrasovaa merged commit 2869b8a into OSGeo:main Mar 26, 2024
@petrasovaa petrasovaa deleted the r.horizon-json branch March 26, 2024 19:43
@petrasovaa petrasovaa mentioned this pull request Jun 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nilason nilason nilason approved these changes

Assignees

@petrasovaa petrasovaa

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

Milestone

8.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@petrasovaa @nilason @neteler