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.surf.volcano: port from grass6 addons shell script #1202

Open
wants to merge 6 commits into
base: grass8
Choose a base branch
from

Conversation

HamishB
Copy link
Contributor

@HamishB HamishB commented Sep 8, 2024

I find this module quite useful for creating synthetic surfaces for testing a variety of other modules.

@neteler
Copy link
Member

neteler commented Sep 8, 2024

Nice!
Do you see a chance to also add a small test?

MN: for addons, the full URL to core modules is needed here

Co-authored-by: Markus Neteler <[email protected]>
@HamishB
Copy link
Contributor Author

HamishB commented Sep 8, 2024

Nice! Do you see a chance to also add a small test?

Hi Markus,

What do you like that to look like? Just some dummy values to see if it completes with exit code 0 or some r.univar or 'r.stats -Ac nsteps=10' analysis to see that the output is within expected bounds?

@neteler
Copy link
Member

neteler commented Sep 8, 2024

Yes, something simple would be great. This quite helps to be able to quickly test also other changes which might indirectly affect this addon.

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.

Happy to see this one back.

# # FIXME: ok, it isn't really kurtosis but it's similar and I couldn't
# # think of a better name.
# %Option
# % key: kurtosis
Copy link
Member

Choose a reason for hiding this comment

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

What about slope_steepness?

sigma = options["sigma"]
kurtosis = options["kurtosis"]

tmp_base = "tmp__rsv_%d" % os.getpid()
Copy link
Member

Choose a reason for hiding this comment

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

gs.append_node_pid will work on clusters.

Comment on lines +89 to +90
from grass.script import core as gc
from grass.script import raster as gr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
from grass.script import core as gc
from grass.script import raster as gr
import grass.script as gs

is what is usually used for simplicity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants