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

grass.jupyter: Allow Users to view/update computational region #3838

Merged
merged 19 commits into from
Jul 31, 2024

Conversation

29riyasaxena
Copy link
Contributor

@29riyasaxena 29riyasaxena commented Jun 15, 2024

Hello Everyone,

This pull request introduces a new feature to grass.jupyter.interactivemap.py: a "View/Update Computational Region" button that allows users to update the computational region by adjusting its boundaries interactively. Users can move the rectangle representing the current computation region and adjust its size by changing its vertex.

Here's how it works:

  1. Activate the "Draw Computational Region" button.
  2. This action will display the current computation region as a rectangle.
  3. Adjust the computation region by moving the rectangle to a new location or resizing it.
  4. Click on "Update Region" to apply the changes.
  5. Deactivate the button by toggling it off.

For a visual demonstration, please watch the following recording:

computational_region_button_working.mp4

@github-actions github-actions bot added Python Related code is in Python libraries notebook labels Jun 15, 2024
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

I spent some time looking into this and I think what we should do is this:
Create a toggle button and when activated, we add an editable Rectangle. Upon untoggling it will save the changed region. It may be nicer to have a button to save it dynamically showed, when you activate the region mode, but we can leave that for later. The code for this:

from ipyleaflet import Map, Rectangle

polygon = Rectangle(
    bounds=((52, 63), (53, 67)),
    color="red",
    fill_color="red",
    rotation=False,
    draggable=True,
    transform=True
)

m = Map(center=(42.5531, -48.6914), zoom=6)
m.add(polygon);

m

So we won't use the DrawControl, we will use it later for drawing geometries we want to save as vector map.

Couple more notes below:

When drawing I am getting:

TypeError: handle_draw() got multiple values for argument 'action'

When I press Save button, I get:

TypeError: save_region() takes 0 positional arguments but 1 was given

I don't see any reprojection here, you need to reproject the rectangle to get the proper coordinates.

From Ipyleaflet documentation:

The DrawControl is deprecated and will be removed in a future release. Please use GeomanDrawControl instead.

@petrasovaa
Copy link
Contributor

Also it should probably zoom to the region when you activate it.

@29riyasaxena
Copy link
Contributor Author

29riyasaxena commented Jun 23, 2024

I spent some time looking into this and I think what we should do is this: Create a toggle button and when activated, we add an editable Rectangle. Upon untoggling it will save the changed region. It may be nicer to have a button to save it dynamically showed, when you activate the region mode, but we can leave that for later. The code for this:

from ipyleaflet import Map, Rectangle

polygon = Rectangle(
    bounds=((52, 63), (53, 67)),
    color="red",
    fill_color="red",
    rotation=False,
    draggable=True,
    transform=True
)

m = Map(center=(42.5531, -48.6914), zoom=6)
m.add(polygon);

m

So we won't use the DrawControl, we will use it later for drawing geometries we want to save as vector map.

Couple more notes below:

When drawing I am getting:

TypeError: handle_draw() got multiple values for argument 'action'

When I press Save button, I get:

TypeError: save_region() takes 0 positional arguments but 1 was given

I don't see any reprojection here, you need to reproject the rectangle to get the proper coordinates.

From Ipyleaflet documentation:

The DrawControl is deprecated and will be removed in a future release. Please use GeomanDrawControl instead.

Hi Anna, I tried various ways to allow users to draw rectangle using ipyleaflet.Rectangle but it looks like it can only be done if the user gives the coordinates, and user is not able to draw anything on the map with this rectangle.

@petrasovaa
Copy link
Contributor

petrasovaa commented Jun 24, 2024 via email

@29riyasaxena
Copy link
Contributor Author

On Sun, Jun 23, 2024, 7:30 PM Riya Saxena @.> wrote: Hi Anna, I tried various ways to I spent some time looking into this and I think what we should do is this: Create a toggle button and when activated, we add an editable Rectangle. Upon untoggling it will save the changed region. It may be nicer to have a button to save it dynamically showed, when you activate the region mode, but we can leave that for later. The code for this: from ipyleaflet import Map, Rectangle polygon = Rectangle( bounds=((52, 63), (53, 67)), color="red", fill_color="red", rotation=False, draggable=True, transform=True ) m = Map(center=(42.5531, -48.6914), zoom=6) m.add(polygon); m So we won't use the DrawControl, we will use it later for drawing geometries we want to save as vector map. Couple more notes below: When drawing I am getting: TypeError: handle_draw() got multiple values for argument 'action' When I press Save button, I get: TypeError: save_region() takes 0 positional arguments but 1 was given I don't see any reprojection here, you need to reproject the rectangle to get the proper coordinates. From Ipyleaflet documentation: The DrawControl is deprecated and will be removed in a future release. Please use GeomanDrawControl instead. Hi Anna, I tried various ways to allow users to draw rectangle using ipyleaflet.Rectangle but it looks like it can only be done if the user gives the coordinates, and user is not able to draw anything on the map with this rectangle.
User would not be drawing the rectangle, the rectangle would be displayed automatically, showing the current computational region. The coordinates of the region will have to be transformed to lat lon.

— Reply to this email directly, view it on GitHub <#3838 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZFVKG6PA6G4PHRWZ7FVYTZI4A27AVCNFSM6AAAAABJL5EK32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGIYTIMBTHE . You are receiving this because you commented.Message ID: @.
>

Just to ensure, the task is to allow users to draw the computational region? I'll just push the changes as of now.

@petrasovaa
Copy link
Contributor

It looks like there is still some misunderstanding. Let me try to explain better what I described above so that we can progress with this.

This is how I envision this to work from the point of view of a user:

  1. You display map (InteractiveMap) with some data.
  2. There is a toggle button (with an icon, for example some rectangle).
  3. You toggle it
  4. A rectangle shows up (could be red to match the color in GUI), this rectangle shows the current computational region.
  5. This rectangle is editable, you can modify it by dragging its corners or move it (but you can't rotate it).
  6. Once you edit it, you untoggle the button and that triggers modifying the computational region based on the modified rectangle. The rectangle disappears.

You will use the ipyleaflet.Rectangle to achieve that, see the code above. You will need to reproject the coordinates of the rectangle from the original CRS to latlon and then the new coordinates from latlon to the original CRS. Note the original region is a rectangle, after transformation it may be rotated, so you need to take a bounding box of it and similarly when transforming the modified rectangle back to the original CRS.

If the user doesn't modify the rectangle, don't modify the computational region. Ideally, you could dynamically add a save button next to the toggle button, so when you toggle, the save button shows up, when you untoggle it, it would disappear. But let's leave this for later.

Don't use the DrawControl.

Let me know if this still needs some clarifications.

@29riyasaxena
Copy link
Contributor Author

It looks like there is still some misunderstanding. Let me try to explain better what I described above so that we can progress with this.

This is how I envision this to work from the point of view of a user:

  1. You display map (InteractiveMap) with some data.
  2. There is a toggle button (with an icon, for example some rectangle).
  3. You toggle it
  4. A rectangle shows up (could be red to match the color in GUI), this rectangle shows the current computational region.
  5. This rectangle is editable, you can modify it by dragging its corners or move it (but you can't rotate it).
  6. Once you edit it, you untoggle the button and that triggers modifying the computational region based on the modified rectangle. The rectangle disappears.

You will use the ipyleaflet.Rectangle to achieve that, see the code above. You will need to reproject the coordinates of the rectangle from the original CRS to latlon and then the new coordinates from latlon to the original CRS. Note the original region is a rectangle, after transformation it may be rotated, so you need to take a bounding box of it and similarly when transforming the modified rectangle back to the original CRS.

If the user doesn't modify the rectangle, don't modify the computational region. Ideally, you could dynamically add a save button next to the toggle button, so when you toggle, the save button shows up, when you untoggle it, it would disappear. But let's leave this for later.

Don't use the DrawControl.

Let me know if this still needs some clarifications.

Oh, okay. I thought the user began by drawing the rectangle. One more thing, how should I define the original computational region?

@petrasovaa
Copy link
Contributor

Oh, okay. I thought the user began by drawing the rectangle. One more thing, how should I define the original computational region?

I know that's how we thought about this before, but I tried to explain that in my initial comment in this PR.

This solution is better because it also allows user to visualize current computational region. Also it looks like you can't have multiple draw controls, so we will instead use the draw controls for drawing vector geometries that can be then saved as a vector map.

You don't define computational region, it is set by the user, you can get the current computational region with gs.region().

@29riyasaxena
Copy link
Contributor Author

I can't figure out why the output_widget is not updated when the user saves the region. Also, I'm aware of the fact that I need to convert the coordinates to CRS; I wanted to first display the coordinates in terms of latlon correctly.

@github-actions github-actions bot added the tests Related to Test Suite label Jul 3, 2024
@29riyasaxena 29riyasaxena force-pushed the draw_computational_region branch from 69213e0 to a9d5490 Compare July 11, 2024 17:15
@petrasovaa
Copy link
Contributor

The zoom-in should not be happening every time user edits the rectangle, it creates a weird flashing.

@petrasovaa
Copy link
Contributor

@29riyasaxena could you please update the description of this PR too? Thanks!

@petrasovaa petrasovaa marked this pull request as ready for review July 20, 2024 20:10
@29riyasaxena
Copy link
Contributor Author

29riyasaxena commented Jul 21, 2024

@29riyasaxena could you please update the description of this PR too? Thanks!

Hi Anna, I am getting the following errors after pulling the changes from the updated code:

  1. I can't see the save region button.
  2. I get this error:
TypeError                                 Traceback (most recent call last)
TypeError: InteractiveMap.draw_computational_region.<locals>.save_region() takes 0 positional arguments but 1 was given

Could you please confirm if things are okay from your side?
If not, should I fix the code first, as I couldn't get a working screenshot of this feature?

@echoix
Copy link
Member

echoix commented Jul 21, 2024

Is it possible to see a little bit more of the error (traceback), including line numbers if possible? It's hard to know what called this function with the wrong number of arguments

@29riyasaxena
Copy link
Contributor Author

traceback

Is it possible to see a little bit more of the error (traceback), including line numbers if possible? It's hard to know what called this function with the wrong number of arguments

Hi @echoix, this error was resolved when I made a local correction by adjusting the save_region function from save_region() to save_region(_) (it's been called on save_button.on_click(save_region)), but Since Anna has been handling this PR due to some ongoing issues I've been experiencing, I wanted to check with her first before committing those changes if she had some more changes or is it a setup issue from my side.

@echoix
Copy link
Member

echoix commented Jul 21, 2024

The underscore (probably) can't really be used like most Python projects here, as it is used for gettext translation. So you are in fact passing in an argument that is a callable (like a lambda), by passing a function without calling it. (It can be called inside that function by using the argument name, adding the parentheses and any parameters inside the parentheses).

The point is, the underscore (probably) isn't a discard variable in this context, and is neither the previous result like in interactive Python

@29riyasaxena
Copy link
Contributor Author

The underscore (probably) can't really be used like most Python projects here, as it is used for gettext translation. So you are in fact passing in an argument that is a callable (like a lambda), by passing a function without calling it. (It can be called inside that function by using the argument name, adding the parentheses and any parameters inside the parentheses).

The point is, the underscore (probably) isn't a discard variable in this context, and is neither the previous result like in interactive Python

Thank you for your explanation @echoix. I think this is the reason why the function wasn't behaving as expected.

@29riyasaxena 29riyasaxena changed the title grass.jupyter: Allow Users to draw computational region grass.jupyter: Allow Users to view/update computational region Jul 22, 2024
@petrasovaa petrasovaa self-requested a review July 31, 2024 16:41
@petrasovaa petrasovaa added this to the 8.5.0 milestone Jul 31, 2024
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

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

I think this is ready to merge.

It occurred to me that part of this functionality (displaying region) could be done with folium as well. I would leave that for a separate PR.

@petrasovaa petrasovaa merged commit afc6079 into OSGeo:main Jul 31, 2024
28 checks passed
Mahesh1998 pushed a commit to Mahesh1998/grass that referenced this pull request Sep 19, 2024
…#3838)

This pull request introduces a new feature to grass.jupyter.interactivemap.py: a "View/Update Computational Region" button that allows users to update the computational region by adjusting its boundaries interactively. Users can move the rectangle representing the current computation region and adjust its size by changing its vertex.

---------

Co-authored-by: Anna Petrasova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries notebook Python Related code is in Python tests Related to Test Suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants