grass icon indicating copy to clipboard operation
grass copied to clipboard

grass.jupyter: Allow Users to draw computational region

Open 29riyasaxena opened this issue 1 year ago • 8 comments

Hi there,

This is the very basic idea of how I am thinking of implementing the feature to allow users to draw computational regions and save them for later purposes. I would love to hear your suggestions on what can be done.

29riyasaxena avatar Jun 15 '24 19:06 29riyasaxena

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

petrasovaa avatar Jun 16 '24 10:06 petrasovaa

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.

29riyasaxena avatar Jun 23 '24 17:06 29riyasaxena

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 https://github.com/OSGeo/grass/pull/3838#issuecomment-2185214039, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZFVKG6PA6G4PHRWZ7FVYTZI4A27AVCNFSM6AAAAABJL5EK32VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBVGIYTIMBTHE . You are receiving this because you commented.Message ID: @.***>

petrasovaa avatar Jun 24 '24 05:06 petrasovaa

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.

29riyasaxena avatar Jun 24 '24 09:06 29riyasaxena

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.

petrasovaa avatar Jun 25 '24 05:06 petrasovaa

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?

29riyasaxena avatar Jun 25 '24 07:06 29riyasaxena

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().

petrasovaa avatar Jun 26 '24 06:06 petrasovaa

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.

29riyasaxena avatar Jun 29 '24 18:06 29riyasaxena

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

petrasovaa avatar Jul 11 '24 19:07 petrasovaa

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

petrasovaa avatar Jul 20 '24 20:07 petrasovaa

@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?

29riyasaxena avatar Jul 21 '24 15:07 29riyasaxena

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

echoix avatar Jul 21 '24 15:07 echoix

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.

29riyasaxena avatar Jul 21 '24 16:07 29riyasaxena

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

echoix avatar Jul 21 '24 16:07 echoix

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 avatar Jul 21 '24 17:07 29riyasaxena