ManiVaultStudio / core

A Flexible and Extensible Visual Analytics Framework for High-Dimensional Data
https://www.manivault.studio/
GNU Lesser General Public License v3.0
9 stars 1 forks source link

Consistent API: Changing a dataset and notifying the core #263

Open alxvth opened 1 year ago

alxvth commented 1 year ago

It is not clear when to manually notify the core after changing a data set and when not.

I think the use of all dataset->set... functions should be consistent.

ThomasKroes commented 1 year ago

Hi Alexander, in principle I would say yes. But in some cases, there is a legitimate reason not to call the core->notifyDatasetChanged automatically (where the plugin developer wants control over when the notification is triggered). Unlike in the case of a GUI re-name, this is generally quite a trivial call. I do see your point though, perhaps we can alter the signature of the setData(...) in such a way that it removes the ambiguity? Perhaps setData(..., bool notify = false)? When there is a need to call the notification automatically, the notify boolean should be set to true. We could do this for other functions that trigger notifications perhaps?

alxvth commented 1 year ago

Yes, that's probably a good solution.

I also agree, that it would be useful to have such a notify boolean in the other functions that require core notifications as well. This would also serve as documentation/reminder for the user that the notification functions exists.

Similar actually, I think it would make sense to have a dataset->setSelection function that complements dataset->getSelection. Currently, you have to do something like

dataset->getSelection<Points>()->indices.assign(newSelection.cbegin(), newSelection.cend());
_core->notifyDatasetSelectionChanged(dataset);

A wrapper that implements the above and is called like dataset->setSelection(newSelection, /*notify=*/ true) would make for more readable code.

ThomasKroes commented 1 year ago

Great idea! I think the selection update mechanism could use a revision. It should be clear to the developer what the intended behavior is; either through a clear function signature and/or proper documentation.