napari-clusters-plotter icon indicating copy to clipboard operation
napari-clusters-plotter copied to clipboard

Move `add_column_to_layer_tabular_data` upstream

Open haesleinhuepf opened this issue 3 years ago • 4 comments
trafficstars

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?

haesleinhuepf avatar Apr 30 '22 15:04 haesleinhuepf

@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.

Cryaaa avatar Apr 30 '22 15:04 Cryaaa

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 avatar May 02 '22 14:05 jo-mueller

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.

jo-mueller avatar May 02 '22 15:05 jo-mueller

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?

haesleinhuepf avatar May 07 '22 09:05 haesleinhuepf