saalfeldlab / n5

Not HDF5
BSD 2-Clause "Simplified" License
158 stars 22 forks source link

The recent bump to version 4.0.0 breaks compatibility with existing plugins and applications #114

Open krokicki opened 6 months ago

krokicki commented 6 months ago

When I create an N5 using the latest version of Fiji it sets the container version to 4.0.0. These new containers are rejected by any software that is not using the latest version of the N5 API. This isn't ideal, especially for less technically inclined users who are relying on Fiji to export N5's which are compatible with other existing Fiji plugins, pipelines, etc.

For example, if I try to load my new N5 using the current RS-FISH plugin I get this error:

  java.io.IOException: Incompatible version 4.0.0 (this is 2.2.1).
    at org.janelia.saalfeldlab.n5.N5FSReader.<init>(N5FSReader.java:121)
    at org.janelia.saalfeldlab.n5.N5FSReader.<init>(N5FSReader.java:136)
    at net.preibisch.rsfish.spark.SparkRSFISH.call(SparkRSFISH.java:123)
    at net.preibisch.rsfish.spark.SparkRSFISH.call(SparkRSFISH.java:32)
    at picocli.CommandLine.executeUserObject(CommandLine.java:1853)
    at picocli.CommandLine.access$1100(CommandLine.java:145)
    at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2255)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2249)
    at picocli.CommandLine$RunLast.handle(CommandLine.java:2213)
  log4j:at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2080)
  log4j:at picocli.CommandLine.execute(CommandLine.java:1978)
  log4j:at net.preibisch.rsfish.spark.SparkRSFISH.main(SparkRSFISH.java:338)

Manually changing the version in attributes.json back to 2.2.1 fixes the issue, so there is no real incompatibility with my data.

Some ideas for potential mitigations:

Thanks for considering!

StephanPreibisch commented 6 months ago

An additional idea would be to expose the boolean checkVersion flag (https://github.com/saalfeldlab/n5/blob/59c21afbfa4c30ad5a008d3a1f87cb25aad2064a/src/main/java/org/janelia/saalfeldlab/n5/N5KeyValueReader.java#L93), but it would require an updated release for n5 2.2.1 ... but moving forward maybe something you could consider. I assume there will more compatibility issues for cloud storage than filesystem storage, so as long as you know what you are doing it might be ok as Konrad pointed out.

axtimwalde commented 6 months ago

The version check currently does what it is supposed to do, indicating that there are incompatibilities expected. We added a new datatype which breaks things. So, I would argue that this is expected. Can you update the consuming software to use the updated libraries? For convenience, we will look deeper into what the Fiji plugins currently do and consider writing old version N5s if we do not use any new features. We will not release an intermediate n5-2.2.1 patch for not checking versions.

StephanPreibisch commented 6 months ago

I really like exporting old versions if new features are not used. With respect to checkVersion, I didn't phrase it correctly ... I agree, releasing an old version of n5 is not desirable, but you might want to consider exposing it moving forward (from now on).

StephanPreibisch commented 6 months ago

We will update RS-FISH-Spark, but you know, Spark projects ... dependencies are always a lot of fun

axtimwalde commented 6 months ago

I bet updating the dependencies in your projects will be a lot easier than hooking in a format specific version downgrade in the Fiji plugins, because it's NOT the right version. I believe the issue was more about if this kind of versioning and response to it is actually good. I think it is because it is very in your face, you immediately know that things are wrong and how to fix them. Some stuff may work with old code but it may also not work. We can decide to not use this information and let users run into issues. We cannot fix things in the past though.

StephanPreibisch commented 6 months ago

Good point. Maybe the right hack is what @krokicki did ... rename the version and see if it still works. Then you really know what you do.

Maybe that is what the exception could say? java.io.IOException: Incompatible version 4.0.0 (this is 2.2.1). You may try to change the version in the n5 by hand and it may or may not work. - or something like that, just to give users that cannot fix it by themselves in any other way a chance ...

StephanPreibisch commented 6 months ago

What is a weird is that the newest pom-scijava 37.0.0 comes with n5-3.0.2 which creates 3.0.0 n5's, while Fiji ships n5-3.1.1 that creates 4.0.0 n5's. Thus I have to manually overwrite the n5 version in each current project to be able to read 4.0.0 n5's ...

axtimwalde commented 6 months ago

Yeah, that's completely normal for artifacts managed by pom-scijava, they are always ahead, including at breakpoints :). The update is already merged into main https://github.com/scijava/pom-scijava/pull/254/files and will be available with the next release. There are still some test incompatibilities and @bogovicj and @cmhulbert have discovered and fixed some bugs, so we will release bugfixes this week, and update pom-scijava again.

StephanPreibisch commented 6 months ago

I always assumed it is the other way around, that pom-scijava should be ahead of what's on the main Fiji update site ... at least that's how it was in the past often.

axtimwalde commented 6 months ago

pom-scijava and Fiji are decoupled.