Skip to content

Conversation

@imagejan
Copy link
Member

This PR adds a Service that allows to open and save org.scijava.table.Table objects.
TableIOService#open() explicitly returns Table objects, so it can be used instead of IOService whenever strict types (i.e. stricter than Object at least) are desired.

Fixes #7.

@haesleinhuepf, please let me know if this would satisfy your needs discussed in imagej/tutorials#77 (comment). (If yes, we should update the howtos accordingly, of course.)

@haesleinhuepf
Copy link
Member

Hey @imagejan ,

greeat, thanks for spending time on this! It looks very good. However, I just cloned the code and tried if/how one can read from such a table. I extended your test to this:

@Test
	public void testTableIOService() {
		String tableFile = "fakeTableFile.csv";
		GenericTable table = new DefaultGenericTable(2, 2);
		table.set(0,0, 1);
		table.set(0,1, 2);

		TableIOService tableIOService = context.getService(TableIOService.class);
		assertTrue(tableIOService.getClass().equals(DefaultTableIOService.class));
		assertTrue(tableIOService.canOpen(tableFile));
		assertTrue(tableIOService.canSave(table, tableFile));


		try {
			// this line throws a java.lang.UnsupportedOperationException
			// tableIOService.save(table, tableFile);
			
			Table<?, ?> data = tableIOService.open(tableFile);
			assertTrue(Table.class.isAssignableFrom(data.getClass()));

			// this line throws a java.lang.IndexOutOfBoundsException
			System.out.println("Cols: " + data.getColumnCount());
			System.out.println("Rows: " + data.getRowCount());

			System.out.println(data.get(0, 1));

		}
		catch (IOException exc) {
			fail(exc.toString());
		}
	}

Either it crashes when saving the table (commented out line) or while accessing the table after reading it. I'm wondering - shouldn't it also crash in case the .csv file cannot be found?

Thanks for your efforts!

Cheers,
Robert

@imagejan
Copy link
Member Author

imagejan commented Nov 15, 2019

Hi @haesleinhuepf,

sorry that I didn't provide enough information here.

Since the actual IOPlugin for reading and saving lives in scijava-plugins-io-table and we don't depend on this component (it would be a circular dependency), I had to rely on a very basic FakeTableIOPlugin that doesn't implement the actual IO, so your changes are indeed expected to fail. That is, we really only test the features of the Service here, all actual IO tests are already present and live in scijava-plugins-io-table.

In order to test this PR in Fiji, you'll have to have both scijava-table-0.4.1-SNAPSHOT and scijava-plugins-io-table-0.2.0.jar on your classpath. Then you can get the service in a script as such:

#@ TableIOService io

What's still missing to make usage convenient from notebooks, is an integration into the gateway, such as ij.table().io().open(...), but that would need to be added to imagej/imagej probably.

This commit adds a Service that allows to open and save org.scijava.table.Table objects.
TableIOService#open() explicitly returns Table objects, so it can be used instead of IOService whenever strict types are desired.

Fixes #7. Dedicated to Robert Haase.
@imagejan imagejan force-pushed the add-table-io-service branch from 250ec53 to 5979e59 Compare November 18, 2019 15:45
Copy link
Member

@haesleinhuepf haesleinhuepf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Cool. Let's merge this.

Just a comment: I don't understand the architecture. I see here code for a TableIOService that cannot save tables. But apparently it does it's job.

@imagejan
Copy link
Member Author

@haesleinhuepf the IOPlugin for Tables existed already before this PR, and lives in scijava-plugins-io-table:

https://github.com/scijava/scijava-plugins-io-table/blob/6312051e4dd805131e5f6eb1d901351eb88c25ef/src/main/java/org/scijava/table/DefaultTableIOPlugin.java

The goal of this PR was to introduce a Service that guarantees that the return type of its open() method is Table (similar to DatasetIOService in SCIFIO). But as @ctrueden explained in #7 (comment), "New services should not live in -plugins- components".

As we have a fully extensible framework, it's enough if the service finds a suitable IOPlugin at runtime, so the implementation of TableIOService can live in this component here (upstream of the -plugins- repository in the dependency hierarchy).

Hope that clarifies things a little : )

Using PluginInfo.create available in scijava-common 2.81.0, we can avoid having the FakeTableIOPlugin test class visible in all tests.
@imagejan imagejan merged commit c6b82b5 into master Dec 20, 2019
@imagejan imagejan deleted the add-table-io-service branch January 9, 2020 17:35
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.

Add TableIOService to read/write tables from locations

3 participants