Skip to content
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

Add Command to save Tables #9

Merged
merged 1 commit into from
Feb 21, 2020
Merged

Add Command to save Tables #9

merged 1 commit into from
Feb 21, 2020

Conversation

imagejan
Copy link
Member

@imagejan imagejan commented Dec 20, 2019

This adds a File > Export > Table... command.
See scijava/scijava-ui-swing#37.

/cc @ctrueden @tferr @adaerr

(In order to test this, you'll also need to have scijava-plugins-io-table-0.2.0.jar on your classpath.)

@adaerr
Copy link

adaerr commented Jan 6, 2020

Thanks for this fix. Back from vacation (happy new year btw, all the best for 2020 !) I wanted to test this but failed so far. A naive

fiji --cp Fiji.app/jars/scijava-table-0.2.0.jar

is not enough to make the File > Export > Table... command appear. I don't know if it's because --cp appends to the classpath (can one prepend somehow?) or some other reason. Worse, I couldn't find where the current GenericTable class is loaded from even by recursively scanning all jars in Fiji.app/. Any help appreciated, otherwise I'll just let the others comment on this PR and wait for it to reach me through a regular update.

@imagejan
Copy link
Member Author

imagejan commented Jan 6, 2020

@adaerr all files in Fiji.app/jars are on the classpath already, so you don't need to add them explicitly. You should however make sure that both scijava-table-0.4.1-SNAPSHOT.jar (from this PR) and the mentioned scijava-plugins-io-table-0.2.0.jar are in your jars directory.

You can run mvn clean install -Dscijava.app.directory=/path/to/your/Fiji.app to install the SNAPSHOT jar in your local Fiji installation.

If you don't manage to get the project built, let me know and I can upload the jar file.

@adaerr
Copy link

adaerr commented Jan 6, 2020

Thanks @imagejan, I didn't think of updating scijava-table (where the scan mentioned above did find GenericTable.class, but my eyes and/or brain must still have been on vacation when looking at the output :-o ). I'll build the 0.4.1-SNAPSHOT right away.

@adaerr
Copy link

adaerr commented Jan 6, 2020

Hum, after having maven install scijava-table-0.4.1-SNAPSHOT.jar and scijava-plugins-io-table-0.2.1-SNAPSHOT.jar in place of the older jars (some other jars added/updated along the way: scijava-common-2.81.0.jar, parsington-1.0.4.jar, eventbus-1.4.jar), I still do not have File > Export > Table... in the Fiji main window menu. Plugins > Utilities > ImageJ Properties seems to confirm I all paths point to the correct Fiji.app directory, so I am at a loss.

@imagejan
Copy link
Member Author

imagejan commented Jan 6, 2020

Hm, what do you see if you type "Table..." in the search bar? And does searching for "Image..." bring up (among others) the File > Import > Image... command (that's also a SciJava Command)?

Oh, and are you sure you checked out the branch of this PR (export-table-command) and didn't build from master?

@adaerr
Copy link

adaerr commented Jan 6, 2020

A "Table..." search brings up legacy:ij.plugin.SimpleCommands (File > Import > Table...) and other unrelated commands (AmiraTable, Scriptable load/save HDF5[..], etc).

"Image..." brings up command:io.scif.commands.SaveDataset (File > Export > Image...) and command:io.scif.commands.OpenDataset (File > Import > Image...) next to the legacy:ij.plugin.Commands("new") (File > New > Image) and some "Image Moment" commands and ops, so Fiji does appear to see some SciJava commands.

@adaerr
Copy link

adaerr commented Jan 6, 2020

How is the menu entry set ? In the scifio jar for instance, META-INF/json/org.scijava.plugin.Plugin contains

{"class":"io.scif.commands.SaveDataset","values":{"menu":[{"label":"File","weight":0.0},{"label":"Export"},{"label":"Image... "}],"type":"org.scijava.command.Command"}}

but the same json file in the scijava-plugins-io-table jar contains only some priority related data:

{"class":"org.scijava.table.DefaultTableIOPlugin","values":{"priority":-100.0,"type":"org.scijava.io.IOPlugin"}}

Is my understanding correct that this metadata is sourced by ImageJ to know how if and where in the menu to put commands ? If yes, how is this file generated ? A brute case-insensitive grep for "export" in my scijava-plugins-io-table git repository clone yields no result. In particular there is no src/main/resources/plugins.config.

@imagejan
Copy link
Member Author

imagejan commented Jan 6, 2020

In my Fiji installation, it does show up just fine.

My suspicion is that you still somehow didn't get my changes from this PR in your build. Can you please try this jar file (rename the zip to jar):

scijava-table-0.4.1-SNAPSHOT.zip

@adaerr
Copy link

adaerr commented Jan 6, 2020

Yes, with your jar the command shows fine in the menu. My two first tries to save a table as foo.txt or foo.csv failed with a

[ERROR] io.scif.FormatException: No compatible output format found for extension: foo.txt

though, so I'll maybe have to update some other components like scifio as well. Getting forward anyway.

FWIW, the recursive diff between the scijava-table-0.4.1-SNAPSHOT jars generated by maven from the git repository, and yours, is

$> diff -r git/ jan/
diff -r git/META-INF/json/org.scijava.plugin.Plugin jan/META-INF/json/org.scijava.plugin.Plugin
1c1
< {"class":"org.scijava.table.DefaultTableDisplay","values":{"type":"org.scijava.display.Display"}}{"class":"org.scijava.table.io.DefaultTableIOService","values":{"type":"org.scijava.service.Service"}}{"class":"org.scijava.table.process.ResultsPostprocessor","values":{"priority":-9999.0,"type":"org.scijava.module.process.PostprocessorPlugin"}}
\ No newline at end of file
---
> {"class":"org.scijava.table.process.ResultsPostprocessor","values":{"priority":-9999.0,"type":"org.scijava.module.process.PostprocessorPlugin"}}{"class":"org.scijava.table.io.DefaultTableIOService","values":{"type":"org.scijava.service.Service"}}{"class":"org.scijava.table.io.ExportTableCommand","values":{"menu":[{"label":"File","mnemonic":"f","weight":0.0},{"label":"Export"},{"label":"Table..."}],"type":"org.scijava.command.Command"}}{"class":"org.scijava.table.DefaultTableDisplay","values":{"type":"org.scijava.display.Display"}}
\ No newline at end of file
diff -r git/META-INF/MANIFEST.MF jan/META-INF/MANIFEST.MF
10c10
< Implementation-Date: 2020-01-06T13:04:43+0000
---
> Implementation-Date: 2019-12-20T12:52:23+0000
13c13
< Implementation-Build: c6b82b5b52e671e0542101ad70cce406b3aaea34
---
> Implementation-Build: d84ab53b457c4887843676ef1c277ca8f41b9e36
diff -r git/META-INF/maven/org.scijava/scijava-table/pom.properties jan/META-INF/maven/org.scijava/scijava-table/pom.properties
1,3c1,3
< version=0.4.1-SNAPSHOT
< groupId=org.scijava
< artifactId=scijava-table
---
> version=0.4.1-SNAPSHOT
> groupId=org.scijava
> artifactId=scijava-table
Only in jan/org/scijava/table/io: ExportTableCommand.class

The pom.properties stuff is only whitespace, but the last line in seems suspicious. I'll have to stop here for today.

@adaerr
Copy link

adaerr commented Jan 6, 2020

Oh, my bad, sorry @imagejan. I saw a recent merge (c6b82b5
scijava/add-table-io-service) in the git history and assumed that this
was the current PR being already commited to master. Checking out the right
branch yields a working jar.

Working in the sense of having a File > Export > Table... command,
that is. Still stuck with the FormatException quoted above when
attempting a table export with it. Will try updating scifio.

@adaerr
Copy link

adaerr commented Jan 6, 2020

And another "silly me": the export works just fine with scifio-0.37
despite throwing an exception - seeing the latter I had not bothered
to check for the exported file. Upgrading to scifio-0.38.3-SNAPSHOT gets
rid of the exceptions, most likely due to scifio commit f8d6868. And causes some other trouble with my PendentDrop plugin that I'll have to address, but that's a different chapter.

In any case sorry for all the noise, TL;DR is: PR works fine for me and appears to create correct CSV output for the PendentDrop plugin's output table. Thanks for the work!

@imagejan
Copy link
Member Author

imagejan commented Jan 7, 2020

@adaerr glad it works for you!

Regarding txt files, the problem is that they can contain anything, not just tables. While saving tables to txt files is not a problem, opening txt files as tables might not always work, depending on the content. So I don't know whether we should support very general file extensions apart from csv.

See also this comment (originally by @lnyng) in the source code:
https://github.com/scijava/scijava-plugins-io-table/blob/6312051e4dd805131e5f6eb1d901351eb88c25ef/src/main/java/org/scijava/table/DefaultTableIOPlugin.java#L118-L122

@adaerr
Copy link

adaerr commented Jan 7, 2020

@imagejan regarding support for txt tables, arguably the support for image export to avi files should be dropped as well then, as these can contain anything (codec-wise), not just uncompressed images. Opening avi files might not always work, depending on the content.

More seriously, I agree that txt as table format is ill-defined, but I'd say that exporting (as opposed to saving) is an operation where the user takes her responsibilities in choosing the target format. If the File > Save command were overloaded to save tables then forcing CSV may make sense because of the expectation that the file can be reopened.

Then again there is little added value in providing txt table export except if the field separator could be specified to be a TAB or a space for quick cut/sort/awk/... hacks using Unix tools. It is rumored that people opting for such strange workflows do exist ;-)

@imagejan imagejan merged commit dd7fcfd into master Feb 21, 2020
@imagejan imagejan deleted the export-table-command branch February 21, 2020 19:44
@ctrueden
Copy link
Member

Thanks @imagejan and @adaerr. Glad this is moving forward.

@adaerr wrote:

Upgrading to scifio-0.38.3-SNAPSHOT gets rid of the exceptions, most likely due to scifio commit f8d6868. And causes some other trouble with my PendentDrop plugin that I'll have to address, but that's a different chapter.

I just wanted to chime in here that I'm currently working on restoring backwards API compatibility for scifio 0.38.x. There were some IMHO unnecessary breaking changes with methods being removed or changed, which I believe can be restored as deprecated methods to avoid breakage downstream. I released pom-scijava 28.0.0 last week but it still uses scifio 0.37. For pom-scijava 29.0.0, which I'm working on doing now, it will update to scifio 0.38, but I won't release it before unbreaking the API as best I can.

@imagesc-bot
Copy link

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/accessing-bonej2-results-table-via-imagej-matlab/29252/4

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

Successfully merging this pull request may close these issues.

4 participants