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

C++: Complete OME-XML serialisation to and from OME model objects #1540

Merged
merged 40 commits into from
Jan 29, 2015
Merged

C++: Complete OME-XML serialisation to and from OME model objects #1540

merged 40 commits into from
Jan 29, 2015

Conversation

ghost
Copy link

@ghost ghost commented Jan 24, 2015

  • Fixes and additions to the Xerces-C wrappers
  • Fixes and additions to the xsd-fu generator and the C++ templates
  • Additional helper methods and functions added to ome-xml; parsing helpers move complexity out of the Genshi templates into real C++ templates, which is easier to edit and debug
  • Addition of OME-XML output and validation to showinf
  • Addition of model testcase to the ome-xml tests
  • Add helpers to MetadataTools and XMLTools from Java MetadataTools, XMLTools and OMEXMLServiceImpl
  • Revert model to 2013-06 for the 5.1.0 release

Note that the generated source code was changed by the template updates.

--no-rebase


Testing: Unit tests (cpp/test/ome-xml/model) will test all the 2013-06 samples except for one with timestamps we don't currently handle [currently can't store dates prior to the POSIX epoch]; I added a custom version without pre-1970 dates for the time being. Additional OME-XML could be tested by dropping it into the 2013-06 samples directory.

showinf should also display OME-XML metadata for TIFF files. Run (in the build tree):

./bf bin/showinf/showinf --omexml yes --validate yes /path/to/tiff

Or in a downloaded binary build:

cd /path/to/unpacked/zip
export BIOFORMATS_HOME=$(pwd)
export LD_LIBRARY_PATH="${BIOFORMATS_HOME}/lib"
PATH="${BIOFORMATS_HOME}/bin:$PATH"
showinf --help
showinf --omexml yes --validate yes /path/to/tiff

Use DYLD_LIBRARY_PATH on MacOS X in place of LD_LIBRARY_PATH.

Note it is likely to be missing original metadata annotations at this point, but the OME-XML should validate and contain all the correct dimensions, etc. (Multi-plane TIFFs will be timeseries and have SizeT==PlaneCount)

/cc @bramalingam @pwalczysko @emilroz

rleigh-codelibre and others added 30 commits January 24, 2015 14:25
Used to introspect which namespace a given model object belongs to.
Primarily using weak_ptr lock() method to prevent invalid weak_ptrs
from throwing bad_weak_ptr exceptions.
If it's a reference, the reference wrapper goes out of scope,
while returning by value is still really just a copy of the
wrapped DOMElement pointer.
Drop old properties which are no longer functional (bitrot).
Add extra parent properties for use in templates.
Due to the fact that shared_from_this() isn't usable inside
constructors, it's not possible to call the update() method
in a constructor since taking references etc. of the object
being constructed will cause problems.  To avoid this, add
create methods which construct the object, call update(), and
then return a shared_ptr to the object.
Debug output breaks the cmake use of the xsd-fu output.  This output was useless
since it was in the generated sources in any case.  All debug output is now expanded
inline in the generated code.
return asXMLElementInternal(document, nullelem);
{% if klass.name != "OME" %}\
xerces::dom::Element element(document.createElementNS(NAMESPACE, "${klass.name}"));
{% end if %}\
Copy link
Member

Choose a reason for hiding this comment

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

there isn't an else?

Copy link
Author

Choose a reason for hiding this comment

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

No. The OME root element is created with the document.

@qidane
Copy link
Contributor

qidane commented Jan 28, 2015

The .ome files look fine.

Would any of the changes to the xsd-fu files affect the java code generation?
Could reverting it for C++ cause something to revert for Java?

@ghost
Copy link
Author

ghost commented Jan 28, 2015

The xsd-fu changes are restricted to the C++ templates so won't affect Java. The exceptions are the tweaks to remove debug output but this won't affect the generated code.

@joshmoore
Copy link
Member

Looks like everyone's happy. Thanks @rleigh-dundee (oh, and @rleigh-codelibre too).

joshmoore added a commit that referenced this pull request Jan 29, 2015
C++: Complete OME-XML serialisation to and from OME model objects
@joshmoore joshmoore merged commit 74843c7 into ome:develop Jan 29, 2015
@@ -190,26 +192,6 @@ def main(model, opts):
content = content.render(encoding='utf-8')
customAsXMLElementPropertyContent[propertyName] = content
# END Custom templates for asXMLElment()
parents = model.resolve_parents(obj.name)
Copy link
Member

Choose a reason for hiding this comment

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

Doh, perhaps a bit belatedly could you explain why this was removed?

Copy link
Author

Choose a reason for hiding this comment

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

It's not removed; it's moved above so that the resolved parents are accessible in the ancillary template fragments.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I intended to commend on if opts.debug

Copy link
Author

Choose a reason for hiding this comment

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

Ah, OK. In that case this was removed for a few reasons. The cmake build captures stdout to introspect what's being generated, and enabling debugging breaks things horribly by spewing rubbish. Debugging is now restricted to additional information expanded in the templates. Additionally, the debug output is mostly just a copy of what's in the actual generated code, so writing it to a file and also to stdout isn't giving us anything we don't already have. For the structured annoations, this is for debugging a single feature; this is included as a more general property list in the C++ templates, and the same can also be added to the Java templates if desired.

@ghost ghost deleted the cpp-metadata5 branch January 29, 2015 10:44
@sbesson sbesson added this to the 5.1.0-m4 milestone Feb 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants