Skip to content
This repository has been archived by the owner on Jun 3, 2020. It is now read-only.

Configuration refactoring #28

Open
4 tasks
mhugo opened this issue Jan 20, 2020 · 10 comments
Open
4 tasks

Configuration refactoring #28

mhugo opened this issue Jan 20, 2020 · 10 comments

Comments

@mhugo
Copy link
Contributor

mhugo commented Jan 20, 2020

Summary of a small design sprint with @vmora and me, on possible evolutions to the configuration part of QGeoloGIS.

Here are some propositions to resolve some needs:

Save the ordering of columns (resp. rows for timeseries)

Current situation

Columns are stored in different arrays:

  • stratigraphy_config
  • log_measures
  • imagery_data
    So it is not very clear how to order logs vs. stratigraphy or vs. imagery

Proposed change

Have two objects of high-level logs and timeseries that store source definitions as JSON arrays.
It will merge stratigraphy, log_measures and imagery_data into the same common array logs

A new key type allows the distinction between stratigraphy, logs, imagery and so on.

Store the symbology

Add a configuration key symbology that stores the raw QGIS QML symbology of each column / row

Remove the concept of "sub-selection"

The current code is designed to work with some data sources that have multiple measures stored in the same table (e.g. chemical analysis).

It is based on a logic of filtering and grouping on data:

  • the "sub selection" combobox of the data selection dialog scans the source layer for unique values on a given field and allows the user to select one value
  • then the selected value is used to filter data from the source layer

Capture d’écran de 2020-01-20 16-13-14

It was initially thought to allow flexibility in the configuration of data sources when the list of distinct values cannot be defined a priori.

However it adds too much complexity to the code base and does not fit very well with the simple idea of "1 measure = 1 data source".

Proposed change

Remove the concept of "sub selection".

Each single measure should be configured as a separate data source, with the possibility to add a QGIS filter (with the configuration key filter)

Replace layer IDs by (uri, name, provider)

Current situation

The configuration uses layer IDs to reference data sources.
Problem: it makes the configuration tied to a specific QGIS project file.

Proposed change

Change layer IDs to a triple (name, URI, provider)

This will also allow a trivial import / export function to have a separate configuration file (that can be put on git easier)

Merged plots

In order to merge different plots on the same column (or row), the configuration will allow to specify for each column an array of data sources.

See the complete example.

Additional metadata on graphs

  • minimum value,
  • maximum value,
  • font title
  • font size
  • column / row width in pixels

Configuration example

# The "main" layer of points, to be replaced by (name, uri, provider)
"collar_7e814841_4cc9_495a_8ef6_9229fcfffb5e" :
{
                  "id_column": "id",
                  "name_column": "id",
                  # new keys for fonts
                  "font": Helvetica,
                  "font_size": 14,
                  "logs":[
                    {
                       # width (in pixels) of the column
                        "width": 200,
                        "name": "Stratigraphy",
                        # each data has one or more "series"
                        series = [
                        {
                        "provider": "postgres", 
                        "uri":"dbname=....",
                        "type": stratigraphy,
                        "feature_ref_column": "hole_id",
                        "depth_from_column": "from_",
                        "depth_to_column": "to_",
                        "formation_code_column": "comments",
                        "rock_code_column": "code",
                        "formation_description_column": "comments",
                        "rock_description_column": "code",
                        "style": "formation_style.xml",
                        # new key to specify whether a column is visible or not
                        "visible": true
                    }],
                    {
                        "name": "Radiometry",
                        "width": 200,
                        "visible": false,
                        "series": [
                            {
                                "source": "radiometry_9d3b823d_d1b6_47a8_a1c2_dd0895a7262e",
                                "uom": "Gamma",
                                "feature_ref_column": "hole_id",
                                # min and max values
                                "min": null,
                                "max": null,
                                # new filter key
                                "filter":"element='Au'",
                                "type": "instantaneous_log",
                                # serialized QGIS symbology
                                "symbology": "xml_string",
                                "event_column": "from_",
                                "value_column": "gamma",
                                "visible": false
                                # possible addition to blend different plots during the merge
                                "blending_mode": "multiply"
                            }
                            ]

                    },
                    {
                        "name": "Image",
                        "source": "radiometry_9d3b823d_d1b6_47a8_a1c2_dd0895a7262e",
                        "feature_ref_column": "hole_id",
                        "type": "imagery_log",
                        "event_column": "from_",
                        "value_column": "gamma",
                        "visible": false
                    },

                  ],
                  "timeseries": []

Tasks

  • Write the UI part for the configuration of timeseries
  • Add a function to import / export the configuration from / to a project
  • Remove the concept of "subselection"
  • Refactor with the new configuration proposed here
@mhugo
Copy link
Contributor Author

mhugo commented Jan 20, 2020

cc @vmora @troopa81 @delhomer

@troopa81
Copy link
Contributor

Change layer IDs to a triple (name, URI, provider)

I'm -1 on this because it prevents from using the abstraction ability of QGIS, for instance if you want to:

  • filter a database table with a request
  • compute a column with an expression.

Merged plots

In order to merge different plots on the same column (or row), the configuration will allow to specify for each column an array of data sources.

See the complete example.

Not sure to see where there is merged plots in the example. And I think it may have a problem of closing brackets ] (before radiometry for instance?)

I'm ok with all the other points.

@delhomer
Copy link
Contributor

delhomer commented Jan 20, 2020

Not sure to see where there is merged plots in the example. And I think it may have a problem of closing brackets ] (before radiometry for instance?)

Agree with this guess!

EDIT: Oops no, I understand. The formatting is quite confusing. The closing brackets is for the series parameter. Are the merged plots stored into this parameter?

@vmora
Copy link
Contributor

vmora commented Jan 20, 2020

I'm -1 on this because it prevents ...

I don't see why it prevent any of this, we keep qgis layers, it's just that they don't need to be in the project layers and duplication layers is not really a burden memory-wise.

it may have a problem of closing brackets ]

Well, it's more to get and idea of the structure, and yes, there is no example in this of more than one plot in the array, but you get the idea...

@mhugo
Copy link
Contributor Author

mhugo commented Jan 21, 2020

I don't see why it prevent any of this, we keep qgis layers, it's just that they don't need to be in the project layers and duplication layers is not really a burden memory-wise.

Actually, @troopa81 is right, thanks for pointing this. Layers don't need to be in the project, but it is better if you can reuse them. In case you want to reuse a layer that has for instance a filter defined or a virtual field defined (I forgot I am actually using this filtering on a specific imported dataset).

But at the same time, I would really like to be able to "copy / paste" a given configuration to another project. So ... maybe we should keep the current internal configuration with layer ids and add import / export functions that translate layer IDs to their (name, uri, provider) definitions

@troopa81
Copy link
Contributor

But at the same time, I would really like to be able to "copy / paste" a given configuration to another project. So ... maybe we should keep the current internal configuration with layer ids and add import export function that translate layer IDs to their (name, uri, provider) definitions

Seems a good option! Just to be sure to understand, it will be possible to store in you project QGeoloGIS configuration either:

  • a layer id which refers to a layer definition in the project
  • a (uri,provider, name)

In the second scenario, what happen if the user made a modification on the layer (add new virtual field for instance)? When we serialize, do we keep the (name, provider, uri) or do we store the layer id resulting in loading the tuple.

@troopa81
Copy link
Contributor

Well, it's more to get and idea of the structure, and yes, there is no example in this of more than one plot in the array, but you get the idea...

I understand the merged plot are in series array, am I right?

@mhugo
Copy link
Contributor Author

mhugo commented Jan 21, 2020

it will be possible to store in you project QGeoloGIS configuration either:

No, I propose to keep the current configuration format in the project file (only layer IDs).

Two new functions would allow the user to export the configuration stored in the project to a "configuration file" which is exactly the same as the one stored inside the project except layer IDs are replaced by (name, uri, provider). We may add here a warning that says filters and virtual fields will be lost during exports.

An import function would take a "configuration file", load layers from their (name, uri, provider) and store the configuration (based on the new layer IDs) inside the project file.

I understand the merged plot are in series array, am I right?

Yes

@troopa81
Copy link
Contributor

troopa81 commented Jan 21, 2020 via email

@mhugo
Copy link
Contributor Author

mhugo commented Jan 21, 2020

Thanks for your feedback, I've changed the description correspondingly.

I've also changed a bit the json example. "unit of measure", "min" and "max" values should be common to the merged sub plots

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants