Ticket #1020 (closed task: fixed)
Opened 2012-02-27T15:19:20-06:00
Last modified 2012-05-15T12:14:25-05:00
Use ItemIO.BOTH where relevant in plugins
| Reported by: | curtis | Owned by: | bdezonia |
|---|---|---|---|
| Priority: | minor | Milestone: |
|
| Component: | Plugins | Version: | |
| Severity: | serious | Keywords: | |
| Cc: | Martin.Horn@… | Blocked By: | #1157 |
| Blocking: |
Description (last modified by curtis)
Many plugins take a Dataset, DatasetView or Display as input, then mutate it and produce no explicit outputs.
We should be declaring such parameters with type = ItemIO.BOTH (rather than the default of ItemIO.INPUT). In the case of Displays, these plugins should then *not* explicitly invoke display.update(), since the DisplayPostprocessor will do it for all output displays.
This approach is more robust since someone might want to invoke multiple plugins programmatically in a workflow, and only update the display at the end.
This change will be useful for KNIP/KNIME, to more accurately model parameter inputs and outputs.
Change History
comment:2 Changed 2012-05-04T13:47:12-05:00 by bdezonia
Many of these plugins use Dataset::update() instead. Dataset::update() generates an event and sets the dirty flag. Many of them do not call display.update(). Determine what the best behavior is here before going forward.
comment:3 Changed 2012-05-07T15:42:09-05:00 by curtis
I am not sure of the best way to deal with Datasets.
Calling Dataset::update() will set off a chain of events that will eventually call Display::update() on all Displays containing that Dataset.
Currently, declaring the Dataset parameter as ItemIO.BOTH will cause DisplayPostprocessor to generate a second Display::update() for all Displays containing that Dataset, which is redundant and potentially wasteful.
However, it is vital that the Dataset be labeled as ItemIO.BOTH because it *is* also an output of the plugin, and downstream code needs to know that.
I can think of a couple different solutions, but none of them seems obviously and unequivocally best to me:
- All concrete implementations of Display become smart enough that calling update() twice in a row does nothing on the second call, because there is nothing left to do. This would entail more careful tracking of what has changed. It also makes doing a "forced refresh" of the display potentially less intuitive, as we would then need a separate method or toggle for doing that.
- The DisplayPostprocessor somehow becomes smart enough to avoid calling Display::update() on Displays that were already updated during plugin execution, either directly or as part of an event chain, such as when calling Dataset::update(). I am not entirely sure how we would track that though.
There is also another problem: see ticket #1157 for details.
comment:4 Changed 2012-05-14T10:12:07-05:00 by bdezonia
- Milestone changed from imagej-2.0.0-beta3 to imagej-2.0.0-beta2
comment:5 Changed 2012-05-15T10:54:28-05:00 by bdezonia
- Cc Martin.Horn@… added
With 853690ad1f0cdb7308982e915444c942630f39c0 all plugins with read/write @Parameters have been tagged with ItemIO.BOTH