Opened 4 years ago

Last modified 2 years ago

#3538 new task

add r.clip into core modules

Reported by: martinl Owned by: grass-dev@…
Priority: normal Milestone: 7.8.3
Component: Raster Version: svn-trunk
Keywords: r.clip Cc:
CPU: Unspecified Platform: Unspecified

Description

Add G7A:r.clip into core modules. Tasks need to be solved:

  • add support for vector clip input
  • write tests

Attachments (2)

r.clip_patch_for_vector_input.diff (2.1 KB ) - added by Nikos Alexandris 4 years ago.
Patch for r.clip to support for vector input map (and optionally a where clause)
testrclip.py (2.1 KB ) - added by veroandreo 4 years ago.
test for r.clip provided by Sunveer Singh

Download all attachments as: .zip

Change History (21)

by Nikos Alexandris, 4 years ago

Patch for r.clip to support for vector input map (and optionally a where clause)

by veroandreo, 4 years ago

Attachment: testrclip.py added

test for r.clip provided by Sunveer Singh

comment:1 by veroandreo, 4 years ago

I applied the patch and tested the vector and where options. All seems to work as expected.

Sunveer Singh, a former Google Code-In student, provided a test for the module. I also tested and it reports no errors. See the attachment.

What else is missing?

comment:2 by Nikos Alexandris, 4 years ago

Thank you Veronica.

While I was trying to shorten the label/description, I missed the point of the module. The module extracts a portion of a raster map in to a new map. Using either the current region or a vector map (optionally with a where clause).

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

Just my thoughts, Nikos

comment:3 by Nikos Alexandris, 4 years ago

The test does not consider the where option. I tested this option, before submitting the diff. I.e.:

r.clip raster=elevation vect=zipcodes where="ZIPNUM='27601'" output=rclip_test --o

and it worked/works fine for me, that means only the specific feature is extracted/clipped out from the input raster image.

Nikos

in reply to:  2 ; comment:4 by martinl, 4 years ago

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

Last edited 4 years ago by martinl (previous) (diff)

in reply to:  4 ; comment:5 by martinl, 4 years ago

Replying to martinl:

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

It remembers me issue with G7:v.clip label

Extracts features of input map which overlay features of clip map. 

something like

Clips features of input vector map which overlay features of clip map.

We could also sync modules, modify r.clip as below:

  • vector -> clip (required)
  • add -r flag

?

in reply to:  4 comment:6 by Nikos Alexandris, 4 years ago

Replying to martinl:

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

Right, I forgot v.clip. However, the sentence "clips input raster/vector map based on current region or vector map if defined" means that the the clip action alters directly the input map. I think the original author correctly used the wording "Extracts portion...".

in reply to:  5 ; comment:7 by Nikos Alexandris, 4 years ago

Replying to martinl:

Replying to martinl:

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

It remembers me issue with G7:v.clip label

Extracts features of input map which overlay features of clip map. 

something like

Clips features of input vector map which overlay features of clip map.

We could also sync modules, modify r.clip as below:

  • vector -> clip (required)
  • add -r flag

?

Why not. It would make things more clear I guess.

in reply to:  4 comment:8 by veroandreo, 4 years ago

Replying to martinl:

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

+1 for using "clip" instead of "extract" in both G7:v.clip and G7A:r.clip. I think "clip" is clear enough and is a well known operation in GIS.

in reply to:  7 comment:9 by wenzeslaus, 4 years ago

Replying to Nikos Alexandris:

Replying to martinl:

Replying to martinl:

Replying to Nikos Alexandris:

I would (re-)name it to r.extract or reword the label/description of the module to read something like: "Clips out portion from the input map based on either the extent of the current region or a vector map".

-1 r.extract brings more confusion which is not really necessary. r.clip does the same as G7:v.clip - clips input raster/vector map based on current region or vector map if defined.

It remembers me issue with G7:v.clip label

Extracts features of input map which overlay features of clip map. 

something like

Clips features of input vector map which overlay features of clip map.

We could also sync modules, modify r.clip as below:

  • vector -> clip (required)
  • add -r flag

?

Why not. It would make things more clear I guess.

I'm not sure if I understand, why is clip required? The reason I wrote r.clip was getting just the portion of the raster map which in the computational region and clip by vector makes sense too. Does required: clip, -r rule (G7:g.parser) fit with your intention?

comment:10 by wenzeslaus, 4 years ago

Tests: Please, name the test functions with descriptive names and add docstrings if name can't capture all the details. This will be great help for the next person who will be changing the code or looking on why a particular test is not running.

comment:11 by neteler, 4 years ago

What is the state of r.clip - can it be moved to trunk now (i.e., prior to creating the new relbr76)? Thanks

in reply to:  11 comment:12 by wenzeslaus, 4 years ago

Replying to neteler:

What is the state of r.clip - can it be moved to trunk now (i.e., prior to creating the new relbr76)? Thanks

We should make sure it is not shipped as part of a release without a thought-through interface (see the discussion above). I think we settled on r.clip but the rest is rather unclear.

Do we seek consistency with other raster modules and having computational region drive all or most of the operation? Or do we seek consistency with v.clip? Or is it some special case? Does it need vector mask input? A raster one? Or is the mask managed by r.mask sufficient? A where option for the vector? cats or range/values for the raster?

comment:13 by martinl, 3 years ago

Milestone: 7.6.07.6.1

Ticket retargeted after milestone closed

comment:14 by martinl, 3 years ago

Milestone: 7.6.17.6.2

Ticket retargeted after milestone closed

comment:15 by martinl, 3 years ago

Milestone: 7.6.27.8.0

comment:16 by neteler, 3 years ago

Milestone: 7.8.07.8.1

Ticket retargeted after milestone closed

comment:17 by neteler, 3 years ago

Milestone: 7.8.17.8.2

Ticket retargeted after milestone closed

comment:18 by neteler, 2 years ago

Milestone: 7.8.2

Ticket retargeted after milestone closed

comment:19 by neteler, 2 years ago

Milestone: 7.8.3
Note: See TracTickets for help on using tickets.