ecmwf / ecpoint-calibrate

Interactive GUI (developed in Python) for calibration and conditional verification of numerical weather prediction model outputs.
GNU General Public License v3.0
20 stars 8 forks source link

Merging WTs #174

Closed FatimaPillosu closed 3 years ago

FatimaPillosu commented 3 years ago

Hi Anirudha,

the new way to merge the WTs is really good. Thanks for the changes!

However, I noticed that with the new way we cannot merge leaves, only nodes. This is anyway wonderful because it allows us to merge groups of WTs quite quickly. However, if you want to merge only one leaf, it is not possible to do that on the tree but we have to go back to the table and click on the arrows and, as pointed out by @AugustinVintzileos, it can be confusing for users when there are lots of WTs.

Would it be possible to allow also the merge of the leaves? We could merge the leaves as we do now (to the leaf on the left), and remove the arrows in the table. Moreover, we could call the "Merge Children" mode something more generic like "Merge nodes".

However, if some flexibility could be added, it would be nice if, for the first leaf of the node, a merge only with the right WT can be done; for the last WT, a merge only with the left WT can be done; and if we want to merge a WT in the middle, the user could choose whether to merge to the left or to the right. This would allow the user to create a different set of merges that at the moment are not allow.

What do you @onyb?

Cheers,

Fatima

ATimHewson commented 3 years ago

Hi Anirudha,

Yes fully agree with Fatima that the new merging tool is great! And it has undoubtedly allowed Augustin to make a lot of progress that he was struggling to make before (together with the speed issue resolution). I think my preference would be to have just one additional option of merging leaves to the left. And I myself would leave in the merge-by-arrows options in the table, which I really like (even if it is a bit cumbersome with big trees!). The two options could then be complementary in allowing the same 'action' I think. Anyway we can discuss amongst ourselves tomorrow what is the preferred way forward here, for key users.

cheers, Tim

FatimaPillosu commented 3 years ago

Ok, so after discussing this with Tim he showed me that allowing merging to the left and to right won't provide more merging combinations than just merging to the left, so we just keep it as it is. Thus, summarizing:

onyb commented 3 years ago

Fixed in v0.28.0.