Add plugin to create tag set for the highlighting of cell divisions #54

maarzt · 2024-02-15T10:27:50Z

Adds a new entry to Mastodon's "Plugins" menu which is called:
"Tags > Add Tag Set to Highlight Cell Divisions"

When executed the plugin adds a new tag set to the model. The tag set has tags which are added to the spots before cell division and first spots after cell division.

The tag set is meant to be used for 3d visualization purposes. Changing the color of a cell shortly before cell division can attract the users attention and makes it easier for the user to focus on the cell divisions.

Screenshot: new menu entry*

Screenshot: the newly created tag set, shown in a trackscheme window

Adds a new entry to Mastodon's "Plugins" menu which is called: "Tags > Add Tag Set to Highlight Cell Divisions"

stefanhahmann

This PR adds a useful new functionality to Mastodon.

I have minor suggestions regarding the code style.
When testing I encountered 2 different types of exceptions.
I have some suggestions for improving the user experience.

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

stefanhahmann · 2024-02-16T12:44:56Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+	@Override
+	public void cancel( String reason )
+	{
+


You could consider adding a comment, why this method is intentionally empty.

Cf.: https://rules.sonarsource.com/java/RSPEC-1186/

stefanhahmann · 2024-02-16T12:45:47Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+	}
+
+	private TagSetStructure.Tag getTag( boolean dividesBefore,
+			boolean dividesAfter, int startOffset, int endOffset )


Could be on the same line as the line above.

stefanhahmann · 2024-02-16T12:49:17Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetPlugin.java

+
+	private static final String[] KEYS = { "not mapped" };
+
+	private static final Map< String, String > menuTexts = Collections.singletonMap( ID, "Add Tag Set to Highlight Cell Divisions ..." );


I recommend to omit the "..." at the end. This would be more consistent with other function that can be called from the menu, which also do not have the "..."

stefanhahmann · 2024-02-16T12:53:54Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetPlugin.java

+
+	private void run()
+	{
+		commandService.run( CellDivisionsTagSetCommand.class, true, "projectModel", projectModel );


When this is called, on my machine, I get this exception:

[ERROR] Malfunctioning plugin: org.scijava.ui.swing.widget.SwingObjectWidget java.lang.NullPointerException at org.scijava.widget.WidgetModel.getChoices(WidgetModel.java:140) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:101) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:54) at org.scijava.plugin.TypedService.find(TypedService.java:70) at org.scijava.plugin.WrapperService.create(WrapperService.java:64) at org.scijava.widget.AbstractInputHarvester.addInput(AbstractInputHarvester.java:110) at org.scijava.widget.AbstractInputHarvester.buildPanel(AbstractInputHarvester.java:84) at org.scijava.widget.InputHarvester.harvest(InputHarvester.java:67) at org.scijava.ui.AbstractInputHarvesterPlugin.process(AbstractInputHarvesterPlugin.java:74) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834)

However, the expected dialog still opens:

stefanhahmann · 2024-02-16T13:00:37Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+	private ProjectModel projectModel;
+
+	@Parameter( label = "Tag set name" )
+	private String tagSetName = "cell divisions";


From the users view, it is possible to have different tag set names for different settings. However, users may easily forget to do this.

If they do, they end up with this situation:

You could consider adding the timepoints setting to the tagset name automatically so that it is easier for the user afterwards to differentiate between different tagset.

E.g.: "highlight cell division (+/- 3 timepoints)"

Hm, I understand your point. But I would like to wait for user feedback. Having a predictable tag set name is also an advantage. There would also be the question if to include the colors in the tag set name and how, etc. Maybe overwriting existing tag sets is the best thing to do?

stefanhahmann · 2024-02-16T13:02:57Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+	@Override
+	public void run()
+	{
+		projectModel.getBranchGraphSync().sync();


When I run the the plugin on this dataset: https://github.com/mastodon-sc/mastodon-example-data/tree/master/tgmm-mini with these settings:

I get this exception:

Exception in thread "AWT-EventQueue-0" java.lang.IndexOutOfBoundsException: Index 8 out of bounds for length 0 at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:64) at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:70) at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:248) at java.base/java.util.Objects.checkIndex(Objects.java:372) at java.base/java.util.ArrayList.get(ArrayList.java:458) at org.mastodon.views.table.FeatureTagTablePanel$MyTableModel.getValueAt(FeatureTagTablePanel.java:635) at java.desktop/javax.swing.JTable.getValueAt(JTable.java:2706) at java.desktop/javax.swing.JTable.prepareRenderer(JTable.java:5721) at java.desktop/javax.swing.plaf.basic.BasicTableUI.paintCell(BasicTableUI.java:2185) at java.desktop/javax.swing.plaf.basic.BasicTableUI.paintCells(BasicTableUI.java:2087) at java.desktop/javax.swing.plaf.basic.BasicTableUI.paint(BasicTableUI.java:1883) at com.formdev.flatlaf.ui.FlatTableUI.paint(FlatTableUI.java:405) at java.desktop/javax.swing.plaf.ComponentUI.update(ComponentUI.java:161) at java.desktop/javax.swing.JComponent.paintComponent(JComponent.java:797) at java.desktop/javax.swing.JComponent.paint(JComponent.java:1074) at java.desktop/javax.swing.JComponent.paintToOffscreen(JComponent.java:5255) at java.desktop/javax.swing.RepaintManager$PaintManager.paintDoubleBufferedImpl(RepaintManager.java:1643) at java.desktop/javax.swing.RepaintManager$PaintManager.paintDoubleBuffered(RepaintManager.java:1618) at java.desktop/javax.swing.RepaintManager$PaintManager.paint(RepaintManager.java:1556) at java.desktop/javax.swing.RepaintManager.paint(RepaintManager.java:1323) at java.desktop/javax.swing.JComponent._paintImmediately(JComponent.java:5203) at java.desktop/javax.swing.JComponent.paintImmediately(JComponent.java:5013) at java.desktop/javax.swing.RepaintManager$4.run(RepaintManager.java:865) at java.desktop/javax.swing.RepaintManager$4.run(RepaintManager.java:848) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.desktop/javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:848) at java.desktop/javax.swing.RepaintManager.paintDirtyRegions(RepaintManager.java:823) at java.desktop/javax.swing.RepaintManager.prePaintDirtyRegions(RepaintManager.java:772) at java.desktop/javax.swing.RepaintManager$ProcessingRunnable.run(RepaintManager.java:1890) at java.desktop/java.awt.event.InvocationEvent.dispatch$$$capture(InvocationEvent.java:313) at java.desktop/java.awt.event.InvocationEvent.dispatch(InvocationEvent.java) at java.desktop/java.awt.EventQueue.dispatchEventImpl(EventQueue.java:770) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721) at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715) at java.base/java.security.AccessController.doPrivileged(Native Method) at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:85) at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:740) at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:203) at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:124) at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:113) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:109) at java.desktop/java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101) at java.desktop/java.awt.EventDispatchThread.run(EventDispatchThread.java:90) [ERROR] Malfunctioning plugin: org.scijava.ui.swing.widget.SwingObjectWidget java.lang.NullPointerException at org.scijava.widget.WidgetModel.getChoices(WidgetModel.java:140) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:101) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:54) at org.scijava.plugin.TypedService.find(TypedService.java:70) at org.scijava.plugin.WrapperService.create(WrapperService.java:64) at org.scijava.widget.AbstractInputHarvester.addInput(AbstractInputHarvester.java:110) at org.scijava.widget.AbstractInputHarvester.buildPanel(AbstractInputHarvester.java:84) at org.scijava.widget.InputHarvester.harvest(InputHarvester.java:67) at org.scijava.ui.AbstractInputHarvesterPlugin.process(AbstractInputHarvesterPlugin.java:74) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834) [ERROR] Malfunctioning plugin: org.scijava.ui.swing.widget.SwingObjectWidget java.lang.NullPointerException at org.scijava.widget.WidgetModel.getChoices(WidgetModel.java:140) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:101) at org.scijava.ui.swing.widget.SwingObjectWidget.supports(SwingObjectWidget.java:54) at org.scijava.plugin.TypedService.find(TypedService.java:70) at org.scijava.plugin.WrapperService.create(WrapperService.java:64) at org.scijava.widget.AbstractInputHarvester.addInput(AbstractInputHarvester.java:110) at org.scijava.widget.AbstractInputHarvester.buildPanel(AbstractInputHarvester.java:84) at org.scijava.widget.InputHarvester.harvest(InputHarvester.java:67) at org.scijava.ui.AbstractInputHarvesterPlugin.process(AbstractInputHarvesterPlugin.java:74) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at java.base/java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:264) at java.base/java.util.concurrent.FutureTask.run(FutureTask.java) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at java.base/java.lang.Thread.run(Thread.java:834)

I am not sure, if this related to this PR.

I don't think it's related to this PR.

The exception does not appear if I execute the code inside a full Fiji installation. I guess this exception is specific to some version of some scijava artifact.

xulman

it's good, no? :-)
I like it

reminds me I have to finish #29 (very related)

xulman · 2024-02-16T12:49:04Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetPlugin.java

+
+	public CellDivisionsTagSetPlugin()
+	{
+		action = new RunnableAction( ID, () -> {


this is nice, I like the defensive style of it, makes the SW more robust..
(always learning from ya ;-) )

xulman · 2024-02-16T12:58:46Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+	private ColorRGB highlightColor = new ColorRGB( "pink" );
+
+	@Parameter( label = "Timepoints to highlight before and after cell division" )
+	private int n = 3;


this maybe could have got a little longer name... down in the code I realized I had to search what 'n' is... but the code is short, so easy :-)

Sorry, I simply have no idea how to name the variable properly.

xulman · 2024-02-16T13:16:13Z

src/main/java/org/mastodon/mamut/tomancak/divisiontagset/CellDivisionsTagSetCommand.java

+			String label = "T-" + ( i + 1 );
+			tagColors.set( n - i, Pair.of( label, color ) );
+		}
+		for ( int i = 0; i < n; i++ )


this is maybe unnecessarily the same - symmetrical, but it makes no sense to optimize it either

unless you maybe want to offer a transition from mother-default, over division, to daughters-new-default colors
(I could imagine color encoding a generation number.. but that's for some different story)

Yes, the colors are very similar. But having different tags for before and after the cell division allows the user to manually assign a different color before the cell division. If they would want to.

that's a good and very valid point... (to have explicit Tags for the colors before and after the division so that the user can later change them...)

Add plugin to create tag set for the highlighting of cell divisions

421eec2

Adds a new entry to Mastodon's "Plugins" menu which is called: "Tags > Add Tag Set to Highlight Cell Divisions"

maarzt requested review from stefanhahmann and xulman February 15, 2024 10:28

maarzt force-pushed the cell-division-tag-set branch from 2162273 to 421eec2 Compare February 15, 2024 11:18

stefanhahmann requested changes Feb 16, 2024

View reviewed changes

xulman approved these changes Feb 16, 2024

View reviewed changes

stefanhahmann assigned maarzt Feb 16, 2024

maarzt merged commit 421eec2 into master Feb 20, 2024
2 checks passed

stefanhahmann deleted the cell-division-tag-set branch December 3, 2024 12:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add plugin to create tag set for the highlighting of cell divisions #54

Add plugin to create tag set for the highlighting of cell divisions #54

maarzt commented Feb 15, 2024

stefanhahmann left a comment

stefanhahmann Feb 16, 2024

stefanhahmann Feb 16, 2024

stefanhahmann Feb 16, 2024

stefanhahmann Feb 16, 2024

stefanhahmann Feb 16, 2024

maarzt Feb 20, 2024

stefanhahmann Feb 16, 2024

maarzt Feb 20, 2024

maarzt Feb 20, 2024

xulman left a comment

xulman Feb 16, 2024

xulman Feb 16, 2024

maarzt Feb 20, 2024

xulman Feb 16, 2024

maarzt Feb 20, 2024

xulman Feb 20, 2024


		private static final String[] KEYS = { "not mapped" };

		private static final Map< String, String > menuTexts = Collections.singletonMap( ID, "Add Tag Set to Highlight Cell Divisions ..." );

Add plugin to create tag set for the highlighting of cell divisions #54

Add plugin to create tag set for the highlighting of cell divisions #54

Conversation

maarzt commented Feb 15, 2024

stefanhahmann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xulman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment