g.region: do not update WIND file when not needed #1627
Conversation
Changes default behavior to behave more intuitively, now WIND file is written only with certain options (e.g. raster=, n=, ...) and it is left untouched if only printing is needed. Added force flag -o to force writing WIND file if desired. This change help avoid issues with parallel processing. Flag -u still makes sense for some cases (also keeps backward compatibility).
|
Looks good to me. No need to include stdbool.h though, as long as grass/gis.h is included. |
| temp_window = window; | ||
| G_adjust_Cell_head3(&temp_window, 0, 0, 0); | ||
| if (G_put_element_window(&temp_window, "windows", name) < 0) | ||
| G_fatal_error(_("Unable to set region <%s>"), name); | ||
| } | ||
|
|
||
| G_adjust_Cell_head3(&window, row_flag, col_flag, 0); | ||
| if (set_flag) { | ||
| if (flag.force->answer || (update_file && !flag.noupdate->answer)) { |
There was a problem hiding this comment.
I'm not 100% sure I understand the logic here: if any of the conditions that set update_file to true are set and -u is not set, then the file will be updated, whatever the force flag ? Why do we need a force flag then, especially since it is mutually exclusive to noupdate ?
I seem to be misunderstanding something...
There was a problem hiding this comment.
The update_file variable determines the default behavior, so it is true only if options like res=, raster=, etc are used, otherwise it will be false. But there are cases when you want to enforce different behavior. For example you want to only print modified region but not actually update it: g.region res=1 -up
Alternatively, and this would be probably rare, you want to update the region although the new default behavior wouldn't update it, e.g. g.region res=1 -o save=myregion (you want to save the region into a named region and update the current one).
There was a problem hiding this comment.
I'm not 100% sure I understand the logic here: if any of the conditions that set update_file to true are set and -u is not set, then the file will be updated, whatever the force flag ? Why do we need a force flag then, especially since it is mutually exclusive to noupdate ?
I seem to be misunderstanding something...
I think this is correct: with "Force update" flag even a g.region -p -o writes out the file, which g.region -p wouldn't; on the other hand e.g. g.region vector=lakes -u wouldn't make much sense (hence trigger error).
There was a problem hiding this comment.
I added parser rule to avoid cases you mentioned (g.region vector=lakes -u) because they can make no sense and could be misunderstood.
|
@mlennert unless you have any objections I plan to merge this in the next couple days... |
Changes default behavior to behave more intuitively, now WIND file is written only with certain options (e.g. raster=, n=, ...) and it is left untouched if only printing is needed. Added force flag -o to force writing WIND file if desired. This change help avoid issues with parallel processing. Flag -u still makes sense for some cases (also keeps backward compatibility).
|
@petrasovaa should this be backported to G7.8? |
|
No, it changes behavior of g.region, it's for G8. |
Changes default behavior to behave more intuitively, now WIND file is written only with certain options (e.g. raster=, n=, ...) and it is left untouched if only printing is needed. Added force flag -o to force writing WIND file if desired. This change help avoid issues with parallel processing. Flag -u still makes sense for some cases (also keeps backward compatibility).
Fix after g.region was broken with OSGeo#1627 (78d902d). Added test for d flag.
Changes default behavior to behave more intuitively, now WIND file is written only with certain options (e.g. raster=, n=, ...) and it is left untouched if only printing is needed. Added force flag -o to force writing WIND file if desired. This change help avoid issues with parallel processing. Flag -u still makes sense for some cases (also keeps backward compatibility).
Fix after g.region was broken with OSGeo#1627 (78d902d). Added test for d flag.
Fix after g.region was broken with OSGeo#1627 (78d902d). Added test for d flag.
Changes default behavior to behave more intuitively, now WIND file is written only with certain
options (e.g. raster=, n=, ...) and it is left untouched if only printing is needed.
Added force flag -o to force writing WIND file if desired.
This change help avoid issues with parallel processing.
Flag -u still makes sense for some cases (also keeps backward compatibility).
Documentation is missing, will add it soon.
For more discussion on this, see:
https://trac.osgeo.org/grass/ticket/2230