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.

Sign up for GitHub

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

Jump to bottom

grass.jupyter: fix resolution handling for InteractiveMap #2376

Merged
merged 3 commits into from May 20, 2022
Merged

grass.jupyter: fix resolution handling for InteractiveMap #2376

merged 3 commits into from May 20, 2022

Conversation

petrasovaa
Copy link
Contributor

Should address #2369. Now when use_region=True or saved_region, it should reproject the map with the correct resolution. Note that in these cases the resolution is not estimated by r.proj but is estimated by reprojecting the extent and using the same ncols/nrows as in the source region, which might not be the most precise way.

@petrasovaa petrasovaa added the bug Something isn't working label May 16, 2022
@petrasovaa petrasovaa added this to the 8.2.0 milestone May 16, 2022
@wenzeslaus wenzeslaus linked an issue May 17, 2022 that may be closed by this pull request
[Bug] Jupyter: region management in InteractiveMap(): always on full map (resolution) #2369
Closed
@neteler
Copy link
Member

neteler commented May 17, 2022

I have tested this with my larger dataset:

image

Then, when displaying it. I get:

image
image

Checking in parallel what is run underneath, with ps aux | grep proj I see:

r.proj input=opennrw_lidar_bonn_p95_50cm_DOM@bonn_lidar output=opennrw_lidar_bonn_p95_50cm_DOM_bonn_lidar mapset=bonn_lidar location=openNRW_epsg25832 dbase=/home/mneteler/grassdata resolution=0.8094142783201477

What I notice is that I set the computational region to res=2m while r.proj uses resolution=0.809 which is resulting in way too many pixels. Possibly my mistake?

@petrasovaa
Copy link
Contributor Author

Try use_region=True

@neteler
Copy link
Member

neteler commented May 17, 2022

Try use_region=True

Ok, this then changes the resolution (not sure how to inspect the bbox):

r.proj input=opennrw_lidar_bonn_p95_50cm_DOM@bonn_lidar output=opennrw_lidar_bonn_p95_50cm_DOM_bonn_lidar mapset=bonn_lidar location=openNRW_epsg25832 dbase=/home/mneteler/grassdata resolution=3.21940318

And it helps:

image

The question is now what should be the default: use_region=True or not, as currently.
Perhaps catch the error above and in case suggest to the user to add use_region=True?

@neteler neteler mentioned this pull request May 17, 2022
Merged
veroandreo
veroandreo reviewed May 18, 2022
View reviewed changes
Copy link
Contributor

@veroandreo veroandreo left a comment

Choose a reason for hiding this comment

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

I added grammar suggestions in the documentation paragraph :)

python/grass/jupyter/interactivemap.py Outdated Show resolved Hide resolved
python/grass/jupyter/interactivemap.py Outdated Show resolved Hide resolved
python/grass/jupyter/interactivemap.py Outdated Show resolved Hide resolved
wenzeslaus
wenzeslaus approved these changes May 18, 2022
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 code and behavior make sense to me.

@petrasovaa @veroandreo
Apply suggestions from code review
140af07 
Co-authored-by: Veronica Andreo <veroandreo@gmail.com>
@petrasovaa petrasovaa removed the request for review from neteler May 20, 2022 03:20
@petrasovaa petrasovaa merged commit d1def5e into OSGeo:main May 20, 2022
@petrasovaa petrasovaa deleted the fix-interactive-resolution branch May 20, 2022 03:23
petrasovaa added a commit that referenced this pull request May 20, 2022
@petrasovaa
grass.jupyter: fix resolution handling for InteractiveMap (#2376)
0000a98
@wenzeslaus wenzeslaus added this to In progress in Integration with Jupyter Notebooks via automation May 26, 2022
@petrasovaa petrasovaa moved this from In progress to Done in Integration with Jupyter Notebooks May 27, 2022
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
@petrasovaa @ninsbl
grass.jupyter: fix resolution handling for InteractiveMap (OSGeo#2376)
705772b
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
@petrasovaa @ninsbl
grass.jupyter: fix resolution handling for InteractiveMap (OSGeo#2376)
586fffb
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@petrasovaa @neteler
grass.jupyter: fix resolution handling for InteractiveMap (OSGeo#2376)
94985d2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veroandreo veroandreo veroandreo left review comments

@wenzeslaus wenzeslaus wenzeslaus approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
Integration with Jupyter Notebooks
Done
Milestone
8.2.0
Development

Successfully merging this pull request may close these issues.

[Bug] Jupyter: region management in InteractiveMap(): always on full map (resolution)
4 participants
@petrasovaa @neteler @wenzeslaus @veroandreo