ConservationInternational / trends.earth

trends.earth - measure land change
http://trends.earth
GNU General Public License v2.0
107 stars 45 forks source link

Manage of classes for land cover change targets #580

Closed timlinux closed 2 years ago

timlinux commented 2 years ago

imagem

User needs to be able to change the number and names of cover classes similar to this tool

imagem

After changing the user should be able to save and load their cover definitions

timlinux commented 2 years ago

If the user adds more classes, they should be nested under the original 7 classes. The original 7 classes will be fixed and user may only add nested classes. Column names for this tool would differ from the aforementioned tool

imagem

imagem

timlinux commented 2 years ago

@alexbruy also note that the user should be able to nominate a colour for each of the new classes, so that e.g. the charting from John can pick up the color defined here.

timlinux commented 2 years ago

@alexbruy if you have not already started on this item, please let us know and we can assign it to @gkahiu

gkahiu commented 2 years ago

@azvoleff, just to make sure that I understand correctly:

  1. We should allow adding of nested classes (in the window below):

edit_definition

OR

  1. Allow adding additional parent land cover classes in addition to the core 7 ones in the window below. If so, which default values (-1, 0, 1) should we assign on creation?

edit_core_lc

OR

  1. Both 1 and 2?
azvoleff commented 2 years ago

It is the second:

Allow adding additional parent land cover classes in addition to the core 7 ones

The tool to change the parent legend should be located on the trends.earth settings window, just above the "Trends.Earth login information" section (you will need to add a new section there).

The tool will be similar to the edit definition tool we have right now, but should allow users to add or remove rows, to select the color that should be assigned to new parent classes, and should allow users to decide which of the original 7 IPCC classes the new parent legend they have added nests under. For example, it would let them setup a relationship like the below, which adds a new cropland, and a new forest class, to the legend:

image

The tool used to modify the custom user parent legend must also allow resetting back to the default parent legend (the 7 classes, as well as loading/saving legends to json - can use the existing load/save functions for this).

If the parent legend is modified by the user, then the below window used to edit class definition nesting from ESA CCI should change to instead allow users to nest the CCI classes under their new, user-defined, parent legend:

image

In other words, the "Output class" column would now allow the user to select among the classes within their parent legend.

The below window used to define the meaning of different types of land degradation transitions would also change to show whichever classes the user defined in their parent legend:

image

You asked

which default values (-1, 0, 1) should we assign on creation?

I'd suggest we default any transitions created by the new legend to stable (0).

Tagging @boc84 here just as an FYI, and so he can track / make any additions from CCD's side if of interest.

gkahiu commented 2 years ago

Thanks for the detailed explanation. Will revert if I have any additional questions.

gkahiu commented 2 years ago

Added in PR #626

A few implementation notes:

  1. It is dependent on this PR in trends.earth-schema repo;
  2. Upon adding custom land cover classes (by clicking OK), the module updates the land degradation transition matrix and land cover nesting in the plugin's settings so that the land cover algorithm can pick up these definitions;

custom land cover

  1. Clearing the custom classes (and saving the settings) will automatically restore the default transition matrix and land cover nesting respectively;

image

  1. The transition meaning for each custom class is defined as Stable by default since the original cross-parent definition is also Stable as highlighted below;

image

  1. For the land cover legend nesting, each custom class inherits the parent's ESA CCI Land Cover classes and for those children that end up not having a parent - defined through the nesting attribute -then they are removed from the model in order to avoid the this ValidationError from being raised.
azvoleff commented 2 years ago

Thanks @gkahiu. I tried adding two test classes:

image

I would then expect that these should show up on in the transition matrix, but the transition matrix appears unchanged:

image

I may not have explained the needed functionality well enough - when new classes are added they must be nested under the existing 7 parent classes (as you've already setup - that part looks great!). But if any custom classes are defined, they should entirely replace those on the transition matrix (so in this case only "Test class 1" and "Test class 2" should show up on that matrix. Same goes for this screen:

image

If any custom classes are defined those classes and only those classes should show in those dropdowns.

Given the above, it likely makes sense to have the below screen used to setup classes default to showing the initial 7 classes by default, with those classes nested under themselves. This would allow users to easily change names/remove some of them without needing to add each of the default classes in manually.

image

gkahiu commented 2 years ago

Thanks @gkahiu. I tried adding two test classes:

image

I would then expect that these should show up on in the transition matrix, but the transition matrix appears unchanged:

image

The custom classes should show in the transition matrix, see example below:

LC Custom Legend

Most likely then it is not syncing with the transition matrix in the settings. Could you check the logs to see if this message is being printed when you click the Settings dialog's OK button?

Also, if you close and reload the Settings dialog, does it show the test classes you had previously defined?

I may not have explained the needed functionality well enough - when new classes are added they must be nested under the existing 7 parent classes (as you've already setup - that part looks great!). But if any custom classes are defined, they should entirely replace those on the transition matrix (so in this case only "Test class 1" and "Test class 2" should show up on that matrix. Same goes for this screen:

image

If any custom classes are defined those classes and only those classes should show in those dropdowns.

That is the case since the legend nesting in the settings should also be updated as shown below:

LC ESA Classes

Most likely this is also related to syncing.

Given the above, it likely makes sense to have the below screen used to setup classes default to showing the initial 7 classes by default, with those classes nested under themselves. This would allow users to easily change names/remove some of them without needing to add each of the default classes in manually.

image

In this case, how should the system respond if the user wishes to remove all the classes? Should it revert to the original 7?

azvoleff commented 2 years ago

Most likely then it is not syncing with the transition matrix in the settings. Could you check the logs to see if this message is being printed when you click the Settings dialog's OK button?

No, I don't get any log messages on pressing OK. The only related log message I see is if I clear the classes I added and and then reopen the settings dialog, in which case I see No land cover classes in settings.. The classes are retained on closing/opening QGIS as well but still don't populate in the transition matrix

Also, if you close and reload the Settings dialog, does it show the test classes you had previously defined?

Yes

Given the above, it likely makes sense to have the below screen used to setup classes default to showing the initial 7 classes by default, with those classes nested under themselves. This would allow users to easily change names/remove some of them without needing to add each of the default classes in manually. image

In this case, how should the system respond if the user wishes to remove all the classes? Should it revert to the original 7?

Yes

gkahiu commented 2 years ago

I have slightly revised the PR to include additional log messages when saving custom classes. Could you update and send me the log messages when you save the settings? The messages will have a Custom Land Cover - prefix.

I have also updated the related PR in the te_schemas repo to fix an issue of updating land cover classes since the class is marked as Frozen.

azvoleff commented 2 years ago

Thanks. Now I get a ValidationError on loading the "Sub-indicators for SDG 15.3.1" tool after editing the classes. I set the following classes:

image

The classes save fine. Then on loading the SDG tool I get the ValidationError. The body of the exception is: {'_schema': ["Meaning of transition from LCClass(code=1, name_short='Class 1', name_long='Class 1', description=None, color='#8fe142') to LCClass(code=1, name_short='Class 1', name_long='Class 1', description=None, color='#8fe142') is undefined (nodata is LCClass(code=-32768, name_short='No data', name_long='No data', description=None, color='#000000'))."]}

In the logs I see the below when setting up the classes:

2022-05-04T20:25:34 INFO Custom Land Cover - Saving LCCInfo to settings: True 2022-05-04T20:25:34 INFO Custom Land Cover - Cropland class not in matrix settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Cropland class successfully removed in matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Wetland class not in matrix settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Wetland class successfully removed in matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Artificial class not in matrix settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Artificial class successfully removed in matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Other land class not in matrix settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Other land class successfully removed in matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Water body class not in matrix settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Water body class successfully removed in matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Adding Class 1 class to matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Adding Class 2 class to matrix. 2022-05-04T20:25:34 INFO Custom Land Cover - Saved updated matrix to settings. 2022-05-04T20:25:34 INFO Loaded land cover legend nesting definition from D:\Code\LandDegradation\trends.earth\LDMP\data\land_cover_nesting_unccd_esa.json 2022-05-04T20:25:34 INFO Custom Land Cover - Cropland class not in nesting settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Cropland class successfully removed in nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Wetland class not in nesting settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Wetland class successfully removed in nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Artificial class not in nesting settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Artificial class successfully removed in nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Other land class not in nesting settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Other land class successfully removed in nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Water body class not in nesting settings, attempting to remove... 2022-05-04T20:25:34 INFO Custom Land Cover - Water body class successfully removed in nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Adding Class 1 parent class to nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Adding Class 2 parent class to nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 10 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 11 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 12 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 20 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 30 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 40 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 160 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 170 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 180 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 190 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 200 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 201 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 202 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 210 from nesting. 2022-05-04T20:25:34 INFO Custom Land Cover - Removing orphaned class with code 220 from nesting. 2022-05-04T20:25:35 INFO Custom Land Cover - Saved updated lc nesting to settings. 2022-05-04T20:25:42 INFO updating local state...

Then I see the below when I load the SDG tool:

2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:31 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:43 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 1" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary 2022-05-04T20:26:50 INFO "Class 2" not found in translation dictionary

gkahiu commented 2 years ago

Thanks @azvoleff, I have updated the PR as well as the one for schemas. It should be fixed now, let me know if it works as expected.

Given the above, it likely makes sense to have the below screen used to setup classes default to showing the initial 7 classes by default, with those classes nested under themselves. This would allow users to easily change names/remove some of them without needing to add each of the default classes in manually.

image

I have also incorporate the change above by:

In addition, the module will not allow a user to delete all classes. It will ensure that there is at least one class remaining and inform the user to either add a new one then delete the current one, or restore the default UNCCD classes.