BBOX calculation in maps/models.py

[lucernae]

Hi, I just wanted to confirm something, about bbox format in geonode.

I saw it in the code here: https://github.com/GeoNode/geonode/blob/master/geonode/maps/models.py#L228

There are bbox calculations involved to get the largest bbox from several layers, however this part here:

                bbox[0] = min(bbox[0], layer_bbox[0])
                bbox[1] = max(bbox[1], layer_bbox[1])
                bbox[2] = min(bbox[2], layer_bbox[2])
                bbox[3] = max(bbox[3], layer_bbox[3])

Seems to assume that bbox is in the format [xmin,xmax,ymin,ymax]. But, if you look at the Layers models, bbox property is taken from ResourceBase, and if you look at the ResourceBase: bbox is defined as:

@property
    def bbox(self):
        return [self.bbox_x0, self.bbox_y0, self.bbox_x1, self.bbox_y1, self.srid]

I don't know if x0,y0 is bottom left or top left. If it is top left, then the previous bbox calculation makes sense. But I also found in ResourceBase, the following method:

   def set_latlon_bounds(self, box):
        """
        Set the four bounds in lat lon projection
        """
        self.bbox_x0 = box[0]
        self.bbox_x1 = box[1]
        self.bbox_y0 = box[2]
        self.bbox_y1 = box[3]

It assumes that the format is [xmin,max,ymin,ymax] again? Can somebody enlighten me about this?

[afabiani]

It looks like an issue to me also. The assumptions are not always correct. It is possible that some of those methods are not used anywhere on the code. We will need some further inspection. I can try to devote some time to this as soon as I have some free slots.

[lucernae]

Thanks! We're using some of this function in QGIS Server backend when creating a maps. I would be glad to help if possible. By viewing code usage in pycharm and trace it. I found that in geonode.layers.models.pre_save_layer, it treates the bbox as [x0,y0,x1,y1] with x0,y0 in the bottom left and x1,y1 on top right. In this code, the problem might show up when a layer is saved twice (x1, and y0 gets swapped before saving).

[afabiani]

@lucernae theoretically the assumption of bbox == [x0,y0,x1,y1] should be correct and keep coherent everywhere in the code.

I would suggest to update the maps/model accordingly in order to fix this particular issue.

I'm going to inspect the code in order to be sure that there are not further wrong assumptions around.

[afabiani]

I kept bbox == [x0,x1,y0,y1] convention everywhere.

Please, test the PR if you can.

[lucernae]

Hi, can I know the reason why you choose [x0,x1,y0,y1] as default and not [x0,y0,x1,y1]?

[afabiani]

Because this is how the native bbox is stored from the geospatial server.

[afabiani]

@lucernae you think the PR is OK or you request some changes?

[lucernae]

From my side (I mostly work on integrating QGIS Server backend), I saw that mostly bbox is in the form of [x0,y0,x1,y1]. For example leaflet, WMS request, WCS request, all assumes that they received the BBox in this format, and will return extent also in this format. So I went on, assuming this is the default standard and uses some code from geonode which uses this assumptions. Although, seeing your commit there, your WCS request for geoserver uses [x0,x1,y0,y1] (CMIIW), so that is probably the default format for geoserver?

So, the point I tried to make is, from my point of view, there are plenty of external API and library that support this format: [xmin,ymin,xmax,ymax] and I think it is probably better if we follow the convention. But my point of view could be limited, maybe I haven't see the other format: [xmin,xmax,ymin,ymax] being used so extensively in other areas, which is why I need your opinion on this. It is ok, if in fact [xmin,xmax,ymin,ymax] is the majority people uses, we just have to document the code accordingly. But if [xmin,ymin,xmax,ymax] is what mostly people uses, then I would like to have changes if that's possible.

[afabiani]

Hi @lucernae, I see your point. I more or less agree with you. But in this case we are speaking just about how GeoNode model store BBOX info. I kept this convention because it was like this from the beginning. The bbox property on Layer was created to simplify few client requests on a very first version of GeoNode (I guess), but this created confusion.

Long story short, I really don't think that it is important here to store the bbox in a way or another. The really important thing is that it is coherent everywhere.

OWS Requests, except for few versions, do not specify a convention. As an instance WMS wants [x0,y0,x1,y1], while WCS and WFS 2.0+ usually use [x0,x1,y0,y1].

GeoServer can be instructed on the convention to use. By default it uses the suggested by OGC.

@simod @jj0hns0n @ingenieroariel @capooti you want to add something to this? This is something I'd like to fix early, since currently it is a potential source of errors on the GeoNode core.

[simod]

no specific comments, only important thing is that is coherent. Thanks

[afabiani]

@lucernae up to you. If you think it is a strict requirement to have everything [x0,y0,x1,y1], I can have a look at this. This will require more work of course. Otherwise I would stay with this.