This repository has been archived by the owner on Jan 22, 2019. It is now read-only.
Serialise validly named model to XML #52
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Schema has been used to generate XML serialisation/
deserialisation code, based on Cellml_1_2.xsd.
Notes:
- CodeSynthesis XSD used as follows:
./generate.sh
This creates Cellml_1_2.cpp and .h.
- This approach of starting with an essentially blank schema document
is not my preferred approach, but I was outvoted.
My personal preference would have been to work incrementally, starting with the
existing schema for CellML 1.1 and making only the changes that pertain to the
differences between CellML 1.1 and the draft for CellML 1.2.
A demonstration of that was done as shown here:
https://github.com/codecurve/libcellml/blob/04de33845306a6468d389fd8a1be25e7405221e7/src/cellml_1_2.xsd
as well as the commits that followed it.
Test was previously intended to fail.
As per http://stackoverflow.com/a/6847817 Note: this had to be redone, since the CMake refactor, it somehow got lost, see 89c624b
Also, copy semantics better than reference for std::string in this case, I think, since move constructor will be invoked.
This is in order to get Ubuntu build slave result. (This will be reverted once cmake update done on build slave).
This eliminates the warning on Linux that a deprecated header was included.
DRY (Don't Repeat Yourself) principle: set flags in one place and reuse from other locations.
Thanks @agarny for catching that. Note: Warnings are now treated as errors, and build fails.
Note that the in-memory representation of the model is unused. I think this is probably OK, since there is only one way of serialising an empty model anyway.
Default is that warnings are not treated as errors, however Buildbots will have this enabled.
The Ubuntu Buildslave has just been updated to Cmake 3.2.
Thanks to @hsorby for pointing out that having it platform dependent was incorrect. Was originally based on similar condition in OpenCOR.
As suggested by @hsorby.
Test fail is intended, serialiser just ignores name.
…o be target specific, this helps a lot when using CMake to import libraries that use the CMake config mechanism.
|
It would be good if the proposed development process is followed:
rather than including the tests along with the implementation. |
|
I really don't want any dependence on boost in the public libCellML API. |
|
It would be good if changes in existing tests were also committed in separately with an explanation as to why they need to be changed. |
Changes that were asked for
|
@nickerso How do you propose we handle optional values?
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the pull request for #51
Note, it incorporates all the work done on #49, so the diff looks big, but once #49 is merged, the diff should be minimal (again, don't review generated code).