Opened 7 years ago
Last modified 5 years ago
#3538 new task
add r.clip into core modules
Reported by: | martinl | Owned by: | |
---|---|---|---|
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)
Change History (21)
by , 7 years ago
Attachment: | r.clip_patch_for_vector_input.diff added |
---|
comment:1 by , 7 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?
follow-up: 4 comment:2 by , 7 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 , 7 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
follow-ups: 5 6 8 comment:4 by , 7 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.
follow-up: 7 comment:5 by , 7 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
?
comment:6 by , 7 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...".
follow-up: 9 comment:7 by , 7 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.
comment:8 by , 7 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.
comment:9 by , 7 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 , 7 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.
follow-up: 12 comment:11 by , 6 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
comment:12 by , 6 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:15 by , 6 years ago
Milestone: | 7.6.2 → 7.8.0 |
---|
comment:19 by , 5 years ago
Milestone: | → 7.8.3 |
---|
Patch for r.clip to support for vector input map (and optionally a where clause)