[GRASS-dev] Re: [GRASS GIS] #1507: r.in.gdal & others: -e must only modify the current mapset's WIND file

[GRASS GIS]

#1507: r.in.gdal & others: -e must only modify the current mapset's WIND file
---------------------------------------------+------------------------------
 Reporter:  hamish                           |       Owner:  grass-dev@…              
     Type:  defect                           |      Status:  new                      
 Priority:  blocker                          |   Milestone:  6.4.2                    
Component:  Default                          |     Version:  svn-trunk                
 Keywords:  r.in.gdal, r.external, v.in.ogr  |    Platform:  All                      
      Cpu:  All                              |  
---------------------------------------------+------------------------------

Comment(by hamish):

 Replying to [comment:5 neteler]:
 > When you create a new location (e.g. location wizard)
 > without defining the boundaries, you obtain a 0-1
 > sized location.
 > The use these import tools to set the DEFAULT_WIND
 > (not WIND) using the now deleted flag.
 > That was the relevant purpose which I still want to
 > use in future (on command line, in one step).

 (see my comments about that in comment:4)

 2c: It's a common problem; locating on disk + importing a geofile is a bit
 of a round-about way of solving it. It would be useful for having a
 general solution; if it belongs anywhere it belongs in g.region, and that
 already (rightly IMO) kicks you out with a "you need to be in PERMANENT"
 error. ..thus the g.mapset wrapper script & gui-startup suggestions for a
 more automated solution.


 bug? if 0-1,0-1, and new imported map is like s=456000 n=457000, watch
 def_wind.south:

 {{{
     /*
 -------------------------------------------------------------------- */
     /*      Extend current window based on dataset.
 */
     /*
 -------------------------------------------------------------------- */
     if (flag_e->answer) {
         G_get_default_window(&def_wind);

         def_wind.north = MAX(def_wind.north, cellhd.north);
         def_wind.south = MIN(def_wind.south, cellhd.south);
         def_wind.west = MIN(def_wind.west, cellhd.west);
         def_wind.east = MAX(def_wind.east, cellhd.east);

         def_wind.rows = (int)ceil((def_wind.north - def_wind.south)
                                   / def_wind.ns_res);
         def_wind.south = def_wind.north - def_wind.rows * def_wind.ns_res;

         def_wind.cols = (int)ceil((def_wind.east - def_wind.west)
                                   / def_wind.ew_res);
         def_wind.east = def_wind.west + def_wind.cols * def_wind.ew_res;

         G__put_window(&def_wind, "../PERMANENT", "DEFAULT_WIND");
     }
 }}}

 0 is the MIN() of 0,456000, so def_wind.south stays set to 0 after the
 first call to it, so then doesn't it corrupt the def_wind.rows
 calculation??
 same is true for def_wind.west.
 If so how has it survived for years like that without anyone noticing?!

 `g.region -s rast=$map at original_mapset` is looking safer..


 (n.b. conflicting code comment ;-)


 > I suggest to revert the changes and reinstate the original
 > behaviour with the limitation that DEFAULT_WIND can only be
 > modified if the user is in PERMANENT (warning + only WIND
 > change otherwise).

 try r49745. If you like it we can update the help pages and sync with the
 other modules. (I'll leave the question of what to do in v.in.lidar up to
 Markus M)

 I am not a big fan of operations that have special meaning based on
 context, but at least this way mostly does what the user wants most of the
 time: fails safe-ish.

 The use case I had been considering was not 01/01 but saving the step of
 "g.region rast=newly_imported_map" and/or the "why do I get a blank
 display even though the import seemed to go ok?" FAQ.


 > I disagree that this functionality has been removed
 > with a discussion time of less than 48hs.

 fair enough; I'd considered this a priority for inclusion in 6.4.2, and
 with no comments on it so far I didn't want to stall a release for a week
 waiting.
 and I did say "due notice..." :-)

 Glynn:
 > The default window provides information about
 > the data in the location.

 I guess this flag is "grow the current window" not "replace the current
 window", and in general you'd like to grow your location definition
 outwards from your first map as time goes on and your number of maps grow.


 Hamish

-- 
Ticket URL: <https://trac.osgeo.org/grass/ticket/1507#comment:8>
GRASS GIS <http://grass.osgeo.org>