Oslandia / QGeoloGIS

Migrated to: https://gitlab.com/Oslandia/qgis/QGeoloGIS
GNU General Public License v2.0
28 stars 6 forks source link

Ergonomic refactor #16

Closed troopa81 closed 5 years ago

troopa81 commented 5 years ago

This PR implements modification described in issue #14

Contrary of what was previously described:

qgeologis_add_plot

All other existing features are supposedly still working. However there is a problem with the configuration of timeseries named Cote de nappe (mesures manuelles) , it crashes QGIS.

Sorry for the size of the PR...

mhugo commented 5 years ago

You can select one or several feature to be displayed in the widget. It replace the "select an other feature" functionnality available from data_selector

Is the multi selection implemented or not ?

However there is a problem with the configuration of timeseries named Cote de nappe (mesures manuelles) , it crashes QGIS.

Any clue ?

troopa81 commented 5 years ago

Is the multi selection implemented or not ?

Yes

However there is a problem with the configuration of timeseries named Cote de nappe (mesures manuelles) , it crashes QGIS.

Any clue ?

My first guess was that the coordinate generated from the data were too huge but I try a quick fix and it doesn't work. Or maybe it's because of instantenous timeseries (Actually, chemical measures seem to have the same problem).

It crashes here https://github.com/troopa81/QGIS/blob/multithreads_tests/src/core/symbology/qgsmarkersymbollayer.cpp#L1020

troopa81 commented 5 years ago

I complete the PR with new feature, the ability to add a plot configuration and save it to your QGIS project.

qgeologis_add_plot

cc @vmora

mhugo commented 5 years ago

@troopa81 Good job !

First quick review:

Pros:

Cons / Not sure:

I'll then create a branch and work on some of these issues before merging into master

mhugo commented 5 years ago

@troopa81 Regarding my main concerns, and after some more thoughts, I think the following additional tasks would make the plugin usable for the different use cases we have :

mhugo commented 5 years ago

@troopa81 After some more testing and considerations:

I've worked in the branch https://github.com/Oslandia/QGeoloGIS/tree/ergonomic_refactor

I will then merge it into master. I close this ticket.

Thanks !