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.stats.zonal: Add screenshots to manual #2577

Merged
merged 2 commits into from
Feb 20, 2023

Conversation

neteler
Copy link
Member

@neteler neteler commented Sep 13, 2022

Add three screenshots to illustrate module functionality (screenshots are clickable) to

https://grass.osgeo.org/grass83/manuals/r.stats.zonal.html#example

image

TODO: possibly improve HTML code

Add three screenshots to illustrate module functionality
@neteler neteler added the manual Documentation related issues label Sep 13, 2022
@neteler neteler added this to the 8.3.0 milestone Sep 13, 2022
@wenzeslaus
Copy link
Member

I think the best approach to HTML here is, don't use it and use image instead. No need to click on individual images, the whole image is enough. The code to create the exact images is missing in the PR. My latest attempt for a better approach to this is a combination of a notebook, d.explanation.plot addon, and ImageMagick which is what I used in d207d49. The d.explanation.plot is optional, but I find it useful. The notebook resolves the reproducibility and the combination to one image which would be a shell script and possibly an SVG. d.explanation.plot does not solve the text well enough, but perhaps okay in a combination with a figure caption in HTML.

I would be curious of what everyone things about the notebook-based approach to images in HTML documentation.

@neteler
Copy link
Member Author

neteler commented Sep 13, 2022

Good hint, I had forgotten the existance of the d.explanation.plot addon.

# output must be of square extent
d.mon wx0 width=400 height=400
d.explanation.plot a=zipcodes b=elevation c=zipcodes_elev_avg raster_font=Vera

Unfortunately the numbers are no longer plotted (ignore the red lines):
image

Seems to be a new issue in d.rast.num.

@wenzeslaus
Copy link
Member

Check the Known Issues section in the documentation. Perhaps it is one of them. You are using GUI. I used a notebook with grass.jupyter.Map.

@wenzeslaus
Copy link
Member

The new version of d.explanation.plot is in. There is couple of notebooks with examples in the repo since d207d49, for example raster/r.series/graphics_for_description.ipynb.

@neteler
Copy link
Member Author

neteler commented Feb 14, 2023

Just to update the current state, I tried it again:

# output must be of square extent
d.mon wx0 width=400 height=400
d.explanation.plot a=zipcodes b=elevation c=zipcodes_elev_avg raster_font=Vera

but still no useful output:

image

@neteler neteler modified the milestones: 8.3.0, 8.4.0 Feb 14, 2023
@wenzeslaus
Copy link
Member

Please, try the notebook and email it to me if it does not work. See the Notes section. There are some known issues with various combinations of displays and there are perhaps some unknown issues as well.

@neteler
Copy link
Member Author

neteler commented Feb 14, 2023

OK, I have now written a similar jupyter notebook (attached) to generate the desired screenshot:

How to use (for those here not too familiar with it):

  • run in North Carolina sample data session
  • gunzip the attached notebook
  • launch jupyter notebook in this GRASS GIS session in the directory where you stored the .ipynb file

graphics_for_description.ipynb.gz

@wenzeslaus Almost successful now but the size leads to this limit due to spatial resolution leads to:

 WARNING: Current region size: 1350 rows X 1500 cols
         Your current region setting may be too large. Cells displayed on
         your graphics window may be too small for cell category number to
         be visible.
ERROR: Aborting (region larger then 200 rows X 200 cols is not allowed)

Maybe some trick could be added to d.explanation.plot to auto-reduce the resolution? Of course off-topic for this PR, somehow.

@wenzeslaus
Copy link
Member

... region larger then 200 rows X 200 cols is not allowed ... Maybe some trick could be added to d.explanation.plot to auto-reduce the resolution? Of course off-topic for this PR, somehow.

This is really an intentional d.rast.num feature. Resolution and extent are based on user settings, i.e., which cells do you want to see? d.explanation.plot just uses that as is. So, if you want to revise behavior, and I agree it is a good idea, let's start with d.rast.num. On the other hand, if you don't want to show the numbers, then d.explanation.plot needs to be changed.

@neteler
Copy link
Member Author

neteler commented Feb 16, 2023

Fair enough. I have now zoomed to a small spatial subset (5 x 5 pixels) which is anyway better to visualize:

r_stats zonal

PR incl. new jupyter notebook updated accordingly. Thanks for convincing me to switch to this approach, I quite like it.

While at it, I have fixed a labeling error in the r.series related notebook and figure.

@wenzeslaus
Copy link
Member

...zoomed to a small spatial subset (5 x 5 pixels) which is anyway better to visualize

The idea for d.explanation.plot was 3x3 or 5x5 rasters which you can conveniently compute by hand.

...Thanks for convincing me to switch to this approach, I quite like it.

I'm glad. My last updates to d.explanation.plot really made it much more usable. However, what makes it really work well is the notebook, i.e., grass.jupyter. Thanks @chaedri!

While at it, I have fixed a labeling error in the r.series related notebook and figure.

Thanks!

@neteler
Copy link
Member Author

neteler commented Feb 16, 2023

The idea for d.explanation.plot was 3x3 or 5x5 rasters which you can conveniently compute by hand.

OT here: Perhaps worth a user warning in d.explanation.plot when one tries to feed it with (way) more pixels.

@wenzeslaus
Copy link
Member

OT here: Perhaps worth a user warning in d.explanation.plot when one tries to feed it with (way) more pixels.

Isn't the message from d.rast.num sufficient? Perhaps that needs to be improved in some way.

@neteler
Copy link
Member Author

neteler commented Feb 19, 2023

Isn't the message from d.rast.num sufficient? Perhaps that needs to be improved in some way.

Yeah, ok. And it would be for a different PR anyway.

I'd merge this PR then.

@neteler neteler modified the milestones: 8.4.0, 8.3.0 Feb 20, 2023
@neteler neteler merged commit a3d3999 into OSGeo:main Feb 20, 2023
@neteler neteler deleted the r_stats_zonal_screenshots branch February 20, 2023 10:29
@wenzeslaus wenzeslaus changed the title r.stats.zonal manual: add screenshots r.stats.zonal: Add screenshots to manual Jun 6, 2023
neteler added a commit to nilason/grass that referenced this pull request Nov 7, 2023
* r.stats.zonal manual: add figure to illustrate module functionality along with jupyter notebook
* r.series: bugfix for labeling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request manual Documentation related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants