lib: Add new SRTM_percent color table #4608

cmbarton · 2024-10-28T00:02:39Z

This color table uses the color ramp from the srtm_plus color table for terrain to create colors for relative elevation (spread over the range of elevations in a raster map) rather than absolute elevation in meters. This applies srtm colors in a way similar to the way the elevation color table applies them. This color table is especially useful in creating nice looking elevation maps and shading relief maps.

If this seems useful, it could be backported to 8.4.x and 7.8

This color table uses the color ramp from the srtm_plus color table for terrain to create colors for relative elevation (spread over the range of elevations in a raster map) rather than absolute elevation in meters. This applies srtm colors in a way similar to the way the elevation color table applies them. This color table is especially useful in creating nice looking elevation maps and shading relief maps.

neteler · 2024-10-28T18:07:27Z

Would you mind to add a screenshot here? (simply pasting works)

cmbarton · 2024-10-28T18:27:41Z

Would you mind to add a screenshot here? (simply pasting works)

Here are 2 from the new Flagstaff, AZ demo data set (NC does not have much relief). An SRTM DEM and a color-shaded relief map of the same area.

cmbarton · 2024-10-28T20:38:41Z

Following up creating an image for Markus, I'd like to make a slight change in this. What is the best way to do it? Modify the PR or submit a new one?

neteler · 2024-10-28T20:40:04Z

Please modify this pull request.

A slight tweak in the color ramp to have smoother changes of color with relative elevation change. Looks good in both low relief and high relief landscapes.

cmbarton · 2024-10-28T21:23:18Z

Please modify this pull request.

done. Added a new image too

neteler · 2024-10-28T21:37:46Z

For full integration of the new color table, it also needs to be added to https://github.com/OSGeo/grass/blob/main/lib/gis/colors.desc (otherwise it won't show up in r.colors, v.colors, etc.).

cmbarton · 2024-10-28T21:57:17Z

OK. Thanks. Just put a 2nd copy there?

…

____________________ Michael Barton

On Oct 28, 2024, at 2:38 PM, Markus Neteler ***@***.***> wrote: For full integration of the new color table, it also needs to be added to https://github.com/OSGeo/grass/blob/main/lib/gis/colors.desc (otherwise it won't show up in r.colors, v.colors, etc.). — Reply to this email directly, view it on GitHub <#4608 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACENLL6OQZAMBY2IEEOUXHTZ52OC7AVCNFSM6AAAAABQWJMH36VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBSGY4DANRYG4>. You are receiving this because you authored the thread.

neteler · 2024-10-28T22:40:23Z

OK. Thanks. Just put a 2nd copy there?

Essentially you edit the file here in your branch and commit/push it to this branch.

cmbarton · 2024-10-29T15:56:27Z

I'll do that after class today. I guess that will make a 2nd PR

echoix · 2024-10-29T15:59:04Z

Since we normally squash-merge PRs, up until this PR is merged, you can add more commits, merge main into, do anything you'd like, and we can edit the final merge message once done

wenzeslaus

The ds store modification to gitignore needs to be universal to all directories. That might the thing which is better handled in a separate PR.

nilason · 2024-10-29T19:39:31Z

The ds store modification to gitignore needs to be universal to all directories. That might the thing which is better handled in a separate PR.

Yes, that should go into a separate PR. Note that the pattern ".DS_Store" is enough.

It is also good to have this in the global git config:

echo .DS_Store >> ~/.gitignore
git config --global core.excludesfile ~/.gitignore

Add strm_pct and short description to list of colors in colors.desc

lib/gis/colors.desc

petrasovaa

I am not fond of the name "srtm_pct", how about srtm_percent or srtm_relative?

cmbarton · 2024-10-30T16:13:26Z

I'm fine with shortening the description and will see about the suggested alternative. Any guidelines on how short/long?
I originally had srtm_rel or srtm_relative but thought that was not clear. srtm_percent is fine. I was trying to keep it short.
I don't want my .gitignore update to be in the PR. I did not intend for it to be there. So I am happy to take it out. How to do that? I'd like to set it for any future commits but don't understand how to set it for me and not have it go into grass/main. This is not a huge issue but suggestions would be welcome.
Is 1 fork = 1 PR? Perhaps this is where I am confused. I was assuming that I could use my fork for >1 PR. Which is correct?
What do you recommend I do at this point?

If there a way to delete the .gitignore part, is the new color table AND description already in the PR?
Or is the color.desc part still waiting to create a new PR?
It both parts (color table and description) is already in the PR, can I just rename the color table and modify the description locally, commit and push?
Do I have to do something else?

echoix · 2024-10-30T17:35:14Z

Is 1 fork = 1 PR? Perhaps this is where I am confused. I was assuming that I could use my fork for >1 PR. Which is correct?

1 branch (of your fork) = 1 PR. You can continue changing the branch, and each commit pushed once the PR is opened shows up here. You can use your fork for creating multiple PRs, each in a branch.

cmbarton · 2024-10-30T19:03:10Z

Is 1 fork = 1 PR? Perhaps this is where I am confused. I was assuming that I could use my fork for >1 PR. Which is correct?

1 branch (of your fork) = 1 PR. You can continue changing the branch, and each commit pushed once the PR is opened shows up here. You can use your fork for creating multiple PRs, each in a branch.

Thanks much. This explains a lot. So I can make any updates, commit and push, and it will update the PR. Good to know.

Changed the name of the color table from srtm_pct to srtm_percent, and shortened the description of the color table in colors.desc

cmbarton · 2024-10-30T23:23:00Z

I made the recommended changes in the PR. I am happy to remove the .gitignore commit if someone can tell me how. Can/should I just delete it in my fork/branch and commit/push?

echoix · 2024-10-30T23:26:04Z

I made the recommended changes in the PR. I am happy to remove the .gitignore commit if someone can tell me how. Can/should I just delete it in my fork/branch and commit/push?

That would do it. It's better a new commit (rather than rebase+force push) as there are already some review comments. After a force-push, it's hard to follow the comments that were made on commits that don't exist anymore in the PR.

cmbarton · 2024-10-30T23:28:33Z

I made the recommended changes in the PR. I am happy to remove the .gitignore commit if someone can tell me how. Can/should I just delete it in my fork/branch and commit/push?

That would do it. It's better a new commit (rather than rebase+force push) as there are already some review comments. After a force-push, it's hard to follow the comments that were made on commits that don't exist anymore in the PR.

Thanks. Done.

wenzeslaus · 2024-10-31T00:47:02Z

The goal is to remove the changes, not file file. Try git checkout upstream/main -- path/to/file. Upstream may be called differently depending on how you set up your local repo, check git remote -v.

cmbarton · 2024-10-31T05:03:19Z

The goal is to remove the changes, not file file. Try git checkout upstream/main -- path/to/file. Upstream may be called differently depending on how you set up your local repo, check git remote -v.

I can always get this back locally. I just want this PR to go through

wenzeslaus · 2024-10-31T12:13:32Z

The deletion is included in the PR. See the Files tab. That's why the file needs to be restored with checkout.

petrasovaa · 2024-10-31T15:18:11Z

Also, I just modified the PR title, see the instructions:
https://github.com/OSGeo/grass/blob/main/doc/development/github_guide.md#pr-title

checked out from main

This color table uses the color ramp from the srtm_plus color table for terrain to create colors for relative elevation (spread over the range of elevations in a raster map) rather than absolute elevation in meters. This applies srtm colors in a way similar to the way the elevation color table applies them. This color table is especially useful in creating nice looking elevation maps and shading relief maps.

github-actions bot added the libraries label Oct 28, 2024

cmbarton requested review from wenzeslaus, petrasovaa and hmitaso October 28, 2024 00:06

cmbarton added enhancement New feature or request raster Related to raster data processing labels Oct 28, 2024

cmbarton added this to the 8.5.0 milestone Oct 28, 2024

cmbarton and others added 2 commits October 28, 2024 12:51

Merge branch 'OSGeo:main' into main

36301e9

update gitignore

48b6c6b

Update srtm_pct

c5ba9c5

A slight tweak in the color ramp to have smoother changes of color with relative elevation change. Looks good in both low relief and high relief landscapes.

wenzeslaus reviewed Oct 29, 2024

View reviewed changes

cmbarton and others added 2 commits October 29, 2024 12:42

update colors.desc for srtm_pct color table

7a86741

Add strm_pct and short description to list of colors in colors.desc

Merge branch 'OSGeo:main' into main

3f1732d

petrasovaa reviewed Oct 30, 2024

View reviewed changes

lib/gis/colors.desc Outdated Show resolved Hide resolved

petrasovaa reviewed Oct 30, 2024

View reviewed changes

update info for new srtm color table

81d35e2

Changed the name of the color table from srtm_pct to srtm_percent, and shortened the description of the color table in colors.desc

deleted .gitignore from this PR

8c7380e

petrasovaa changed the title ~~Add new SRTM_pct color table~~ lib: Add new SRTM_percent color table Oct 31, 2024

replaced .gitignore

e7098cf

checked out from main

echoix approved these changes Oct 31, 2024

View reviewed changes

petrasovaa approved these changes Nov 1, 2024

View reviewed changes

petrasovaa merged commit 4385b26 into OSGeo:main Nov 1, 2024
26 checks passed

a0x8o mentioned this pull request Nov 6, 2024

E722 a0x8o/grass#21

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: Add new SRTM_percent color table #4608

lib: Add new SRTM_percent color table #4608

cmbarton commented Oct 28, 2024 •

edited

Loading

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024

cmbarton commented Oct 28, 2024

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024 via email

neteler commented Oct 28, 2024

cmbarton commented Oct 29, 2024

echoix commented Oct 29, 2024

wenzeslaus left a comment

nilason commented Oct 29, 2024

petrasovaa left a comment

cmbarton commented Oct 30, 2024

echoix commented Oct 30, 2024

cmbarton commented Oct 30, 2024

cmbarton commented Oct 30, 2024

echoix commented Oct 30, 2024

cmbarton commented Oct 30, 2024

wenzeslaus commented Oct 31, 2024

cmbarton commented Oct 31, 2024

wenzeslaus commented Oct 31, 2024

petrasovaa commented Oct 31, 2024

lib: Add new SRTM_percent color table #4608

lib: Add new SRTM_percent color table #4608

Conversation

cmbarton commented Oct 28, 2024 • edited Loading

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024

cmbarton commented Oct 28, 2024

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024

neteler commented Oct 28, 2024

cmbarton commented Oct 28, 2024 via email

neteler commented Oct 28, 2024

cmbarton commented Oct 29, 2024

echoix commented Oct 29, 2024

wenzeslaus left a comment

Choose a reason for hiding this comment

nilason commented Oct 29, 2024

petrasovaa left a comment

Choose a reason for hiding this comment

cmbarton commented Oct 30, 2024

echoix commented Oct 30, 2024

cmbarton commented Oct 30, 2024

cmbarton commented Oct 30, 2024

echoix commented Oct 30, 2024

cmbarton commented Oct 30, 2024

wenzeslaus commented Oct 31, 2024

cmbarton commented Oct 31, 2024

wenzeslaus commented Oct 31, 2024

petrasovaa commented Oct 31, 2024

cmbarton commented Oct 28, 2024 •

edited

Loading