Minor changes #55
Merged
Minor changes #55
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
The file seems to be malformatted: it is missing meta-info and canvas-information: Added a Log-entry to inform the user (in GUI too) and an IOException with a corresponding message. Should malformatted files like this be parsed in any additional way?
(would previously keep saying that it is reading the file, when it has already stopped doing so) Should empty files even be accepted by the fileFilters?
Can now set the --ignore-validation option to true (default is false), to try converting even if validation fails. If kept as false, and validation fails, conversion will be aborted with a warning, but without an exception.
Species reference glyphs without an assignd role (i.e. null) will now be set to UNDEFINED role, with a warning. For me, issue draeger-lab#51 does not work out the same: Already going from SBML to Escher causes problems.
The file GlycolysisLayout_small.sbml.xml seems entirely dysfunctional: It was missing a speciesGlyph and still backconversion (when ignoring validation) does not work, as Escher2Standard will enter into an infinite while-loop. The program shows correct behaviour if --ignore-validation is not set (i.e. it does not even try to convert, because validation fails).
Added a little warning to the ignore-validation-help-entry
I redid the examplefile (with Escher; content is only approximately equal: Previously, the file contained only the malformatted layout for the first two reactions), now it does no longer cause problems.
(whyever they were deleted in the first place)
draeger
reviewed
Dec 23, 2019
| @@ -228,6 +228,10 @@ public static EscherMap parseEscherJson(InputStream stream) | |||
| // An Escher array contains meta-info about the map as the first object and the actual map as | |||
| // second map. | |||
| JsonNode escherJson = objectMapper.readTree(stream); | |||
| if(escherJson.get(0) == null) { | |||
| logger.severe(format(bundle.getString("EscherConverter.missingMetaInfo"))); | |||
| throw new IOException("Missing or misplaced meta-info: File is malformatted"){}; | |||
There was a problem hiding this comment.
It would be preferable to use the message String from the bundle also for the IOException. Creating a variable of type String first to load the message could help.
Comment on lines
234
to
235
| logger.severe(format(bundle.getString("EscherConverter.missingMetaInfo"))); | ||
| throw new IOException("Missing or misplaced meta-info: File is malformatted"){}; |
There was a problem hiding this comment.
Please reuse the String from the bundle also for the IOException.
data/GlycolysisLayout_small.sbml.xml
Outdated
| @@ -35,46 +35,52 @@ | |||
| <position x="50" y="190"/> | |||
| <dimensions width="270" height="20"/> | |||
| </boundingBox> | |||
| </speciesGlyph> | |||
| <speciesGlyph id="glyph_F6P" species="Fructose-6-phosphate"> | |||
There was a problem hiding this comment.
The identation seems to use too many spaces.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Should fix issues #51 and #52