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

ome-xml: java: Don't report unhandled links prematurely #128

Merged
merged 1 commit into from
Jan 5, 2023
Merged

ome-xml: java: Don't report unhandled links prematurely #128

merged 1 commit into from
Jan 5, 2023

Conversation

rleigh-codelibre
Copy link
Contributor

This PR is a second version of #81 but is Java-only and omits the C++ changes which are no longer applicable. Please also see the discussion on that PR.

This PR corrects some long-standing issues in the Java reference processing implementation.

  • OMEModelObject::link implementations always chained up to their parent before handling the linking themselves. This works, but would result in the parent issuing a warning if it couldn't handle it, despite the child later handling the link. We now chain up only if the child can't handle the link, and issue a warning right at the top if nothing handled it. This solves a number of annoying Java model serialisation warnings which are completely bogus.

In effect, you will see that setting an AnnotationRef on Detector or any other element derived from ManufacturerSpec will no longer have ManufacturerSpec::link issue a bogus warning. This applies to any model object which is derived from another model object, e.g. LightSource, where the reference is handled by the most derived type, or any derived type after the first, where the chaining up will trigger the spurious warning.

For OMERO import, this should remove a large number of the bogus import warnings related to OME-XML metadata which we have seen on e.g. the QA system and other channels over the last few years. LightSource and Shape are notable culprits.

Testing:

  • Run tiffinfo or tiffcomment on test.ome.tiff. Check that you get three AnnotationRef elements linking to the three annotations, demonstrating that the reference processing and subsequent serialisation worked correctly.
  • Check Java with a similar sample for C++ metadata-formatwriter, creating an AnnotationRef on Detector or similar. You'll see the warnings go when built against this ome-model snapshot, but otherwise no other functional changes.

@rleigh-codelibre
Copy link
Contributor Author

Rebased to run with GitHub Actions.

@sbesson sbesson requested a review from dgault November 7, 2022 14:47
Copy link
Member

@dgault dgault left a comment

Choose a reason for hiding this comment

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

Tested with a custom sample file with annotation refs on detector and lightsource. The only change with this PR was in the logging, removing the incorrect warnings. Functionally the behaviour remains unchanged with and without the PR.

@dgault dgault merged commit 3a4b468 into ome:master Jan 5, 2023
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.

2 participants