search

BiAPoL / napari-clusters-plotter

A napari plugin for clustering objects according to their properties.
BSD 3-Clause "New" or "Revised" License
77 stars 10 forks source link

Move `add_column_to_layer_tabular_data` upstream #83

Open haesleinhuepf opened 2 years ago

haesleinhuepf commented 2 years ago

I'm a big fan of this method:

add_column_to_layer_tabular_data

Would anyone mind if we move it to napari-skimage-regionprops? For backwards compatibility we could then import it in utilities like this:

from napari_skimage_regionprops import add_column_to_layer_tabular_data

With this import, no code would have to be changed immediately.

I'm also thinking if this function (or a similar function) would resolve https://github.com/haesleinhuepf/napari-skimage-regionprops/issues/16

Maybe @jo-mueller has an opinion?

Cryaaa commented 2 years ago

@haesleinhuepf I'm all for it! It would fit there better anyway since it is mainly a table specific function! And regarding the issue raised I think this function would circumvent what @jo-mueller was talking about.

jo-mueller commented 2 years ago

Hi @haesleinhuepf @Cryaaa ,

sounds like a good idea to me - another step of taking infrastructure code out of the clusters plotter and making it a bit more lightweight :)

jo-mueller commented 2 years ago

Since we are already discussing about it here - would it be desirable to have this function throw a warning if, for instance, the passed column name was already in the target table? I created a simple implementation here.

haesleinhuepf commented 2 years ago

would it be desirable to have this function throw a warning if, for instance, the passed column name was already in the target table

Yes, I think this is a good idea. Furthermore, how about adding a parameter overwrite_existing with default value False?