Skip to content

Add method to retrieve a list of files #273

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
imagejan opened this issue Jun 29, 2017 · 7 comments
Closed

Add method to retrieve a list of files #273

imagejan opened this issue Jun 29, 2017 · 7 comments

Comments

@imagejan
Copy link
Member

I would like to implement a FilelistPreprocessor that allows to choose a collection of files, such that commands can be annotated as:

@Parameter
File[] fileList

As JFileChooser can be set up to allow multiple file selection, I think it would be useful to add a method File[] chooseFiles(File file, String style) to UIService (and/or UserInterface and LegacyUI), but I fear that changing the interface would entangle quite some changes to the implementing classes downstream, so I wanted to ask about opinions.

@ctrueden would you consider this a useful addition?

Considering the possible styles, I think it would only make sense for open and directory style dialogs, not for save.
Also, this would allow to add some more useful methods that allow providing default filters:

  • File chooseFile(File file, String style, javax.swing.filechooser.FileFilter[] filterList)
  • File[] chooseFiles(File file, String style, javax.swing.filechooser.FileFilter[] filterList)
@imagejan
Copy link
Member Author

imagejan commented Jun 29, 2017

I gave it a shot and added FilelistPreprocessor here (hard-coding JFileChooser and not relying on a UI except for adding the chooser to a Component):

scijava/batch-processor@95292cd

I noticed that now this preprocessor is also called for a bunch of other modules (the third line is my actual test script) 😄:

[WARNING] FilelistPreprocessor called for module: net.imagej.updater.CheckForUpdates
[WARNING] FilelistPreprocessor called for module: org.scijava.ui.swing.script.ScriptEditor
[WARNING] FilelistPreprocessor called for module: org.scijava.script.ScriptModule@49c8127b
[WARNING] FilelistPreprocessor called for module: net.imagej.app.QuitProgram

UPDATE: of course this is intended behavior, I simply forgot to check for null on ModuleItem 😄 The updated commit is here:

scijava/batch-processor@ab9151a

@ctrueden
Copy link
Member

In principle I think this is a great idea. As you suggest, we will need to add more method signatures to the UIService and UserInterface plugin type to support it.

File list parameter

  • Firstly, I'd like to support File[] as well as List<File> parameters.
  • If you have a single File parameter along with other stuff such as e.g. double, the File parameter will be handled by a FileWidget with a text field plus browse button. What should the system do when there is a File[] along with other stuff? Which widget will handle this? That same FileWidget, or a new FileListWidget? And if the latter, how would it be rendered? Note that @tibuch has experimental work on a general-purpose List<?> input widget, which delegates to the widget type of its elements, and provides + and - buttons for adding and removing elements. I think this would be awkward for File objects, though—we probably want a dedicated widget? Or at least make the Browse button support multiple file selection in the same way, then... and somehow list all chosen files in the text field?

File filtering

We cannot use javax.swing.filechooser.FileFilter for the file filtering, as it is Swing-specific. We will unfortunately need to invent our own FileFilter-style class, and each UI will need to adapt the SciJava file filter objects into its own objects internally, similar to what we do for all the UI events. We can add helpers for this to scijava-ui-awt and scijava-ui-swing. The SciJava filter could perhaps extend java.io.FileFilter or java.io.FilenameFilter interfaces, so that our filters can be used e.g. by java.io.File.listFiles(...) methods.

One question is whether we want to limit our design to files here, or try to support org.scijava.io.Location objects more generally; @gab1one has done some work recently which is tangentially related to this topic.

@imagejan
Copy link
Member Author

imagejan commented Jun 30, 2017

@ctrueden wrote:

I'd like to support File[] as well as List<File> parameters.

Would it be sufficient to have a Converter from List<File> to File[] to achieve this (without having to create two distinct method signatures)?

That same FileWidget, or a new FileListWidget? And if the latter, how would it be rendered?

I would prefer a new FileListWidget, possibly providing a Browse button next to a JList field listing the currently selected images, similar to the one implemented in the Image Reader dialog in KNIME, while keeping it as simple as possible:

image

@imagejan
Copy link
Member Author

imagejan commented Jun 30, 2017

I added the chooseFiles method signatures here:

master...imagejan:multiple-file-input

and added implementations for:

Remaining things (to be done and/or discussed):

  • Add List<File> signatures (or converters?)
  • Implement FileListInputPreprocessor that calls chooseFiles on single File[] inputs
  • Implement a FileListWidget that hands off to chooseFiles for each multiple File[] inputs
  • Should the method signature be changed to allow each of the three available modes for JFileChooser.setFileSelectionMode(int mode) via a String style or similar?
    • JFileChooser.FILES_ONLY
    • JFileChooser.DIRECTORIES_ONLY
    • JFileChooser.FILES_AND_DIRECTORIES

@imagejan
Copy link
Member Author

imagejan commented Jul 3, 2017

I made some progress on the above-mentioned branches, and also added Drag-n-Drop functionality to the FileWidget (plan to add added it to FileListWidget (imagejan/scijava-ui-swing@e4f598d) as well, of course) 😄

Currently I use a JList typed on File which gets displayed correctly, but the returned value from the input harvester is a File constructed from the String returned by JList.

@ctrueden if you have comments on how to best fix this, I'd appreciate some advice. Should I simply use JList<String> instead, and construct my File objects from the returned String values?

@imagejan
Copy link
Member Author

imagejan commented Jul 4, 2017

Two comments, before I forget:


@ctrueden wrote:

One question is whether we want to limit our design to files here, or try to support org.scijava.io.Location objects more generally; @gab1one has done some work recently which is tangentially related to this topic.

Supporting locations would be great, but I currently limited the implementation to Files, as drag-and-drop works nicely with them, and supporting more general locations would add quite some complexity.


I'd like to support File[] as well as List<File> parameters.

Currently, any parameter of type List<?> (such as List<String> or List<File>) throws a ClassNotFoundException:

javax.script.ScriptException: Unknown type: List<String>
	at org.scijava.script.DefaultScriptService.lookupClass(DefaultScriptService.java:215)
	at org.scijava.script.process.ParameterScriptProcessor.parseParam(ParameterScriptProcessor.java:202)
	at org.scijava.script.process.ParameterScriptProcessor.parseParam(ParameterScriptProcessor.java:174)
	at org.scijava.script.process.ParameterScriptProcessor.process(ParameterScriptProcessor.java:165)
	at org.scijava.script.process.ParameterScriptProcessor.process(ParameterScriptProcessor.java:134)
	at org.scijava.script.process.ScriptProcessorService.process(ScriptProcessorService.java:79)
	at org.scijava.script.ScriptInfo.parseParameters(ScriptInfo.java:291)
	at org.scijava.module.AbstractModuleInfo.initParameters(AbstractModuleInfo.java:191)
	at org.scijava.module.AbstractModuleInfo.inputList(AbstractModuleInfo.java:153)
	at org.scijava.module.AbstractModuleInfo.inputs(AbstractModuleInfo.java:95)
	at org.scijava.module.process.GatewayPreprocessor.process(GatewayPreprocessor.java:66)
	at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:104)
	at org.scijava.module.ModuleRunner.run(ModuleRunner.java:156)
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:126)
	at org.scijava.module.ModuleRunner.call(ModuleRunner.java:65)
	at org.scijava.thread.DefaultThreadService$3.call(DefaultThreadService.java:237)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.IllegalArgumentException: Cannot load class: List<String>
	at org.scijava.util.ClassUtils.loadClass(ClassUtils.java:217)
	at org.scijava.util.ClassUtils.loadClass(ClassUtils.java:137)
	at org.scijava.script.DefaultScriptService.lookupClass(DefaultScriptService.java:210)
	... 19 more
Caused by: java.lang.ClassNotFoundException: List<String>
	at java.net.URLClassLoader.findClass(URLClassLoader.java:381)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:424)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at org.scijava.util.ClassUtils.loadClass(ClassUtils.java:209)
	... 21 more
[WARNING] Ignoring invalid parameter: List<String> stringList

I think some substantial changes to the input harvesting are required to make it work on lists. I'd like to have a look at @gab1one's implementation before proceeding on this end.

@imagejan
Copy link
Member Author

imagejan commented Oct 3, 2017

Resolved with #286.

@imagejan imagejan closed this as completed Oct 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants