Rewrite of blind-experiment to IJ2 plugin commands#1
Conversation
imagejan
left a comment
imagejan
left a comment
There was a problem hiding this comment.
Great, thanks a lot, @ScienceToolkit!
I'll try to have a more detailed look soon, but already added some first comments.
| * *Plugins > Blind Experiment > Mask filenames* to mask filenames before analysis | ||
| * *Plugins > Blind Experiment > Unmask filenames* to unmask filenames after analysis | ||
| * *Plugins > Blind Experiment > Unmask results table* to unmask labels of a results table | ||
| * *Analyze > Blind Experiment > Blind Files* to mask filenames before analysis |
There was a problem hiding this comment.
Is to blind/unblind the right term, and clear enough to users what it means? I was unsure and chose mask/unmask at the time, but would be fine with whatever you or others think works best.
There was a problem hiding this comment.
I just went with the terminology we have been using around the lab, I will change that if most prefer mask/unmask.
There was a problem hiding this comment.
After thinking about it for a while, I'd still prefer mask/unmask.
| <!-- NewPage --> | ||
| <html lang="en"> | ||
| <head> | ||
| <!-- Generated by javadoc (1.8.0_181) on Thu Sep 06 14:54:08 EDT 2018 --> |
There was a problem hiding this comment.
The javadoc should not be part of the source code, it can be generated with maven any time. How comes that you have it in ./doc/? Maven should put it into ./target/site/apidocs/ (and /target/ is in .gitignore and therefore ignored).
There was a problem hiding this comment.
Ah, this is my bad completely. I was playing around in Eclipse and generated that. I will remove the folder.
| @@ -0,0 +1,105 @@ | |||
| package imagejan.plugins; | |||
There was a problem hiding this comment.
Let's stick with the sc.fiji.blind_experiment package, and move the repository to the fiji org as discussed on the forum.
There was a problem hiding this comment.
Ok, I will refactor that.
imagejan
left a comment
imagejan
left a comment
There was a problem hiding this comment.
Sorry, @ScienceToolkit, that it took me so long to get back to this. I added a few more comments.
My major remaining point now is that we should also enable unmasking files when their folder isn't saved in the prefs (either because some other folder was processed in the meantime, or because the unmasking is done on a different machine).
| <licenses> | ||
| <license> | ||
| <name>Simplified BSD License</name> | ||
| <name>GPL</name> |
There was a problem hiding this comment.
We have to stick to the license names supported by maven-license-plugin: http://www.mojohaus.org/license-maven-plugin/examples/example-license-list.html
Let's change this to gpl_v3.
| * *Plugins > Blind Experiment > Mask filenames* to mask filenames before analysis | ||
| * *Plugins > Blind Experiment > Unmask filenames* to unmask filenames after analysis | ||
| * *Plugins > Blind Experiment > Unmask results table* to unmask labels of a results table | ||
| * *Analyze > Blind Experiment > Blind Files* to mask filenames before analysis |
There was a problem hiding this comment.
After thinking about it for a while, I'd still prefer mask/unmask.
| @Override | ||
| public void run() { | ||
| // Unblind the files using the saved props file | ||
| File propsFile = new File(prefs.get("blindPropsPath")); |
There was a problem hiding this comment.
This leaves no possibility to unmask files/results that were masked earlier in the past. If you masked several folders, only the last one will be saved in the prefs, no?
Let's keep the directory as an input parameter as I had it in my original implementation. We can still use a DynamicCommand with an initializer method to pre-fill the path with the one saved in the prefs.
| Plugins>Blind Experiment, "Unmask results table", imagejan.plugins.Blind_Experiment("unblind_results") | ||
| Plugins>Blind Experiment, "Mask filenames", sc.fiji.blind_experiment.Blind_Experiment("blind_files") | ||
| Plugins>Blind Experiment, "Unmask filenames", sc.fiji.blind_experiment.Blind_Experiment("unblind_files") | ||
| Plugins>Blind Experiment, "Unmask results table", sc.fiji.blind_experiment.Blind_Experiment("unblind_results") |
There was a problem hiding this comment.
Now that all commands are implemented as SciJava Commands, this plugins.config is not needed any more. Let's remove it.
2 participants
I am making changes to the blind-experiment plugin to move it into the IJ2 framework and make it as user-friendly as possible. The following changes were introduced:
The functionality of the blinding and unblinding commands is encased in public methods. I think these should be made static as they could be useful without an instance of the plugin class, but would like some insight into if that is the sensible thing to do.
Also, any comments on how I can make future pull requests better are also appreciated.