Skip to content

Comments

r.horizon: change JSON format#3888

Merged
echoix merged 2 commits intoOSGeo:mainfrom
petrasovaa:r.horizon-JSON-change
Jun 28, 2024
Merged

r.horizon: change JSON format#3888
echoix merged 2 commits intoOSGeo:mainfrom
petrasovaa:r.horizon-JSON-change

Conversation

@petrasovaa
Copy link
Contributor

@petrasovaa petrasovaa commented Jun 18, 2024

Given the discussions about JSON format, I decided to change it to:

[
    {
        "x": 635729.779412,
        "y": 223023.897059,
        "horizons": [
            {
                "azimuth": 0.000000,
                "angle": 3.389152,
                "distance": 270.000000
            },
            {
                "azimuth": 100.000000,
                "angle": 1.945799,
                "distance": 386.393582
            },
            {
                "azimuth": 200.000000,
                "angle": 2.586693,
                "distance": 458.802790
            }
        ]
    }
]

Before it was (#3534):

[
    {
        "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
        ],
        "horizon_distance": [
         ....
        ]
    }
]

Importantly, this needs to get into 8.4 release.

@petrasovaa petrasovaa added blocker Blocking a release backport to 8.4 PR needs to be backported to release branch 8.4 labels Jun 18, 2024
@petrasovaa petrasovaa added this to the 8.4.0 milestone Jun 18, 2024
@petrasovaa petrasovaa self-assigned this Jun 18, 2024
wenzeslaus
wenzeslaus approved these changes Jun 18, 2024
View reviewed changes
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 change in format is good and fits with what we are doing in v.info.

It would be good to have it in the release, so we don't release format which we don't want to use. However, it would be possible to make the change in a backward-compatible way.

@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 Jun 18, 2024
@echoix
Copy link
Member

echoix commented Jun 26, 2024

Is this ready to merge? And the other JSON ones?

@echoix echoix modified the milestones: 8.4.0, 8.5.0 Jun 28, 2024
@echoix echoix merged commit cb0e431 into OSGeo:main Jun 28, 2024
cyliang368 pushed a commit to cyliang368/grass that referenced this pull request Jul 1, 2024
@petrasovaa @cyliang368
r.horizon: change JSON format (OSGeo#3888)
b455cec
@a0x8o a0x8o mentioned this pull request Jul 1, 2024
Merged
a0x8o pushed a commit to a0x8o/grass that referenced this pull request Jul 2, 2024
@petrasovaa @a0x8o
r.horizon: change JSON format (OSGeo#3888)
f30b3bd
a0x8o pushed a commit to a0x8o/grass that referenced this pull request Jul 23, 2024
@petrasovaa @a0x8o
r.horizon: change JSON format (OSGeo#3888)
b3f26fa
@neteler
Copy link
Member

neteler commented Sep 20, 2024

Importantly, this needs to get into 8.4 release.

So far it hasn't been backported. Shall I do that, @petrasovaa ?

petrasovaa added a commit that referenced this pull request Sep 20, 2024
@petrasovaa
r.horizon: change JSON format (#3888)
8c18359
@petrasovaa petrasovaa removed the backport to 8.4 PR needs to be backported to release branch 8.4 label Sep 20, 2024
@petrasovaa
Copy link
Contributor Author

petrasovaa commented Sep 20, 2024

It was my intention to have it in 8.4.0... Thanks for noticing this...

@petrasovaa petrasovaa deleted the r.horizon-JSON-change branch September 20, 2024 14:03
@echoix
Copy link
Member

echoix commented Mar 1, 2025

r.windfetch (addon) needs to be adjusted, as this didn't seem to have been part of 8.4.0, but is now as of recently in 8.4.1. How do we handle these kinds of breaking changes affecting addons? r.windfetch already required 8.4, but the json format to parse from changed in a patch version.

@echoix echoix mentioned this pull request Mar 13, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wenzeslaus wenzeslaus wenzeslaus approved these changes

Assignees

@petrasovaa petrasovaa

Labels

blocker Blocking a release 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.5.0

Development

Successfully merging this pull request may close these issues.

4 participants

@petrasovaa @echoix @neteler @wenzeslaus