scijava / scijava-table

Table structures for SciJava applications
Other
3 stars 1 forks source link

Add TableIOService and test #8

Closed imagejan closed 4 years ago

imagejan commented 4 years ago

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 https://github.com/imagej/tutorials/pull/77#discussion_r344592926. (If yes, we should update the howtos accordingly, of course.)

haesleinhuepf commented 4 years ago

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 commented 4 years ago

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.

imagejan commented 4 years ago

@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 https://github.com/scijava/scijava-table/issues/7#issuecomment-553093466, "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 : )