QuantStack / jupytergis

JupyterGIS
BSD 3-Clause "New" or "Revised" License
15 stars 5 forks source link

Implement schemas #4

Open martinRenou opened 2 weeks ago

martinRenou commented 2 weeks ago

We need to implement some schemas to get started.

A good starting point may be to implement schemas for TileLayer / GeoJSON layer.

martinRenou commented 1 week ago

@davidbrochart @brichet

About our file format, I'm thinking we could do the following:

{
  "layers": {
    "1234-1234-1": {"type": "TileLayer", "name": "My Layer 1", "value": {"url": "https://...jpg"}},
    "1234-1234-2": {"type": "VectorLayer", "name": "My Layer 2", "value": {"source": "5678-5678-1", "styling": {"stroke": "red"}}}
  },
  "dataSources": {
    "5678-5678-1": {"type": "VectorSource", "value": {[GEOJSON-format]}}
  },
  "layerGroups": {
    "group1": {"layers": ["1234-1234-1", "..."], "visible": false},
    "group2": {"layers": ["1234-1234-1", "..."]},
  }
}
davidbrochart commented 1 week ago

Looks good. It seems everything is indexed by UID except for the layer groups? Should data sources be part of the file?

martinRenou commented 1 week ago

It seems everything is indexed by UID except for the layer groups

Right, we should probably indeed have UUIDs for groups too 👍🏽

Should data sources be part of the file?

Indeed it may be better to have data sources outside of the format.

An updated version of the above:

{
  "layers": {
    "1234-1234-1": {"type": "TileLayer", "name": "My Layer 1", "value": {"url": "https://...jpg"}},
    "1234-1234-2": {"type": "VectorLayer", "name": "My Layer 2", "value": {"source": "5678-5678-1", "styling": {"stroke": "red"}}}
  },
  "dataSources": {
    "5678-5678-1": {"type": "GeoJSONFile", "url": "./path/to/local.json"}
  },
  "layerGroups": {
    "9876-9876-1": {"name": "group1", "layers": ["1234-1234-1", "..."], "visible": false},
    "9876-9876-2": {"name": "group2", "layers": ["1234-1234-1", "..."]},
  }
}
brichet commented 1 week ago

Yes, it looks good. It may be better indeed to save the data sources outside. That would avoid duplication if a data file already exist, or if data comes from a URL.

davidbrochart commented 1 week ago

We should look into SpatiaLite.

martinRenou commented 1 week ago

If we go closer to the maplibre design, our format could look like the following:

{
  "layers": {
    "1234-1234-1": {"type": "raster", "name": "My Layer 1", "source": "5678-5678-2"},
    "1234-1234-2": {"type": "line", "name": "My Layer 2", "source": "5678-5678-1", "styling": {"stroke": "red"}}
  },
  "sources": {
    "5678-5678-1": {"type": "geojson", "url": "./path/to/local.json"},
    "5678-5678-2": {"type": "raster", "url": "https://...jpg"},
  },
  "layerGroups": {
    "9876-9876-1": {"name": "group1", "layers": ["1234-1234-1", "..."], "visible": false},
    "9876-9876-2": {"name": "group2", "layers": ["1234-1234-1", "..."]},
  }
}
brichet commented 1 week ago

After discussion with @martinRenou, the groups may be the reference for the display. This means that if some layers are not grouped, we should create a group of one element for each.

If we go in that direction, we should probably set the layerGroups as a list, to keep the groups order (and therefore the z-index).

{
  "layers": {
    "1234-1234-1": {"type": "raster", "name": "My Layer 1", "source": "5678-5678-2"},
    "1234-1234-2": {"type": "line", "name": "My Layer 2", "source": "5678-5678-1", "styling": {"stroke": "red"}}
  },
  "sources": {
    "5678-5678-1": {"type": "geojson", "url": "./path/to/local.json"},
    "5678-5678-2": {"type": "raster", "url": "https://...jpg"},
  },
  "layerGroups": [
   {"name": "group1", "layers": ["1234-1234-1", "..."], "visible": false},
   {"name": "group2", "layers": ["1234-1234-1", "..."]},
  ]
}

What do you think ?

davidbrochart commented 1 week ago

After discussion with @martinRenou, the groups may be the reference for the display. This means that if some layers are not grouped, we should create a group of one element for each.

I'm not sure about that. This means that each time a layer is added, it is also added as a single layer group?

If we go in that direction, we should probably set the layerGroups as a list, to keep the groups order (and therefore the z-index).

Maybe we could keep the layerGroups entry as a map of ID to group, where a group has a z-index attribute?

brichet commented 1 week ago

I'm not sure about that. This means that each time a layer is added, it is also added as a single layer group?

Yes, every layer should be in a group. Otherwise we would have to make distinction between the individual layers and the grouped ones.

Maybe we could keep the layerGroups entry as a map of ID to group, where a group has a z-index attribute?

That works too, I don't have a strong opinion on that.

davidbrochart commented 1 week ago

Yes, every layer should be in a group. Otherwise we would have to make distinction between the individual layers and the grouped ones.

But let's say a user creates a layer group with just one layer. We now have two layer groups with the same layer, and we need to make a distinction that one is an implicit layer group, and the other is an explicit layer group. I'm not sure it is better.

martinRenou commented 1 week ago

We can make this transparent to the user so that the user does not know it's a group with one layer. If the user creates a group with one layer it will mostly be a no-op. It's really just for convenience for us on the display, so we just need to loop over the layer groups to meet every layer.

martinRenou commented 1 week ago

It's like in the QGIS data structure, you have access to the layer tree which contains all layers, which is the equivalent to our list of layer groups.

davidbrochart commented 1 week ago

If the user creates a group with one layer it will mostly be a no-op.

What do you mean by "no-op"? It's a valid group, for instance a user could mark the layer as not visible in this group, or with a special transparency value.

It's like in the QGIS data structure, you have access to the layer tree which contains all layers, which is the equivalent to our list of layer groups.

I think that if we go that road we would need a layer group attribute that would flag this group as implicit, e.g. "is_layer": true. And these implicit layer groups would not be saved to the file, but rather created at runtime. For me it's really a convenience for the UI to treat every layer as a layer group for simplicity.

martinRenou commented 1 week ago

How about this instead:

{
  "layers": {
    "1234-1234-1": {"type": "raster", "name": "My Layer 1", "source": "5678-5678-2"},
    "1234-1234-2": {"type": "line", "name": "My Layer 2", "source": "5678-5678-1", "styling": {"stroke": "red"}},
    "..."
  },
  "sources": {
    "5678-5678-1": {"type": "geojson", "url": "./path/to/local.json"},
    "5678-5678-2": {"type": "raster", "url": "https://...jpg"},
  },
  "layersTree": [
       "1234-1234-1",
       {
           "name": "group1", "layers": [
               "1234-1234-2", 
               {"name": "group11", "layers": ["1234-1234-11", "..."]}
            ], 
            "visible": false
       },
       {"name": "group2", "layers": ["1234-1234-3", "..."]},
   ]
}
brichet commented 1 week ago

And these implicit layer groups would not be saved to the file, but rather created at runtime

If I understand correctly, we would lost the z-index with that, or should have a z-index on the layer itself, which seems a bit confusing (mixing z-index in groups and layers).

martinRenou commented 1 week ago

Yeah I think the z-index is just implicit with the tree/groups structure

davidbrochart commented 1 week ago

I don't quite understand the notion of layer tree.

martinRenou commented 1 week ago

What don't you understand?

brichet commented 1 week ago

How about this instead:

Looks great, we assume that if the entry is a string it is an individual layer, otherwise it is a group. And the layer tree is a list in this schema.

I don't quite understand the notion of layer tree.

The idea is to be able to hide or perform other action on a group of layers.

davidbrochart commented 1 week ago

So in a layer tree a node can be a layer or a layer group, and a layer group has layers that can again be layer groups? That's very confusing. Why aren't these layers a flat list of layers?

brichet commented 1 week ago

Why aren't these layers a flat list of layers?

To allow creating groups. We can think about a scenario using a map and some layers in background, and have several groups displaying data that are not meant to be displayed together. Using groups, you can easily switch between the different data you want to display.

martinRenou commented 1 week ago

QGIS and felt both have the ability to group layers, I'm not sure how felt is structuring its data this but QGIS has a tree of layers in memory and saved into the file.

In felt you can create groups to control visibility or other visual parameters to a set of layers, and to structure a bit more your file:

https://github.com/QuantStack/jupytergis/assets/21197331/3bd3c36c-9fb9-4cb0-9ef7-b2f2378f9bf4

A flat list of layers can quickly be unreadable if you have many layers, so being able to group them sounds like a basic feature to me

davidbrochart commented 1 week ago

I know groups are useful, what I mean is that groups should just consist of a flat list of layers, but not a possibly nested list of layers and groups.

martinRenou commented 1 week ago

Having nested groups would be useful to users too, and it would not bring to much complication for us. So why not do it?

davidbrochart commented 1 week ago

What's the use case, and how do you display nested groups?

martinRenou commented 1 week ago

It looks like felt does not support nested groups, but QGIS does:

Screenshot from 2024-06-21 11-33-31

What's the use case

Well:

display nested groups

I'd expect that reading through the tree would give us a natural z-index?

martinRenou commented 1 week ago

QGIS does

I think that's an important point, if we want to support QGIS files

davidbrochart commented 1 week ago

OK, makes sense. If the UUIDs in the layer tree can be layers or layer groups, let's make sure we don't end up with recursion :smile:

martinRenou commented 1 week ago

Yeah.. We need to make sure the GUI never creates such broken file, but we cannot stop people from manually editing the file and making mistakes. Proper error handling is all we can do.

brichet commented 1 week ago

I'm trying to build the nested schema but this creates an expected object.

IIUC we have:

The tree and group calls each others.

So I added the layerTree in the content:

"layerTree": {
  "$ref": "#/definitions/jGISLayerTree"
},

and in the definitions, I added:

"jGISLayerGroup": {
  "title": "IJGISLayerGroup",
  "type": "object",
  "additionalProperties": false,
  "required": ["name", "layers"],
  "properties": {
    "name": {
      "type": "string"
    },
    "layers": {
      "$ref": "#/definitions/jGISLayerTree"
    },
    "visible": {
      "type": "boolean"
    },
    "parameters": {
      "type": "object"
    }
  }
},
"jGISLayerTree": {
  "title": "IJGISLayerTree",
  "type": "array",
  "default": [],
  "items": {
    "oneOf": [
        {"type": "string"},
        {
          "type": "object",
          "$ref": "#/definitions/jGISLayerGroup"
        }
    ]
  }
}

When building it, I have an additional IJGISLayerGroup1, probably because of recursion.

export type IJGISLayerTree = (string | IJGISLayerGroup)[];
export interface IJGISLayerGroup {
  name: string;
  layers: IJGISLayerTree;
  visible?: boolean;
  parameters?: {
    [k: string]: any;
  };
}
export interface IJGISLayerGroup1 {
  name: string;
  layers: IJGISLayerTree;
  visible?: boolean;
  parameters?: {
    [k: string]: any;
  };
}

Don't know how to properly declare it.

davidbrochart commented 1 week ago

Maybe there should only be LayerGroup, an object with name and layers, which should be a list of strings (individual layer UUID) or layer groups. And there would be an implicit root layer group (maybe with an empty name).

brichet commented 1 week ago

Maybe there should only be LayerGroup, an object with name and layers, which should be a list of strings (individual layer UUID) or layer groups. And there would be an implicit root layer group (maybe with an empty name).

Looks good, thanks