spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r732167785
##########
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
##########
@@ -138,9 +143,11 @@ public void readGraph(final InputStream graphInputStream,
final Graph graphToWri
case GraphMLTokens.DATA:
final String key = reader.getAttributeValue(null,
GraphMLTokens.KEY);
final String dataAttributeName = keyIdMap.get(key);
+ final String defaultValue = defaultValues.get(key);
if (dataAttributeName != null) {
- final String value = reader.getElementText();
+ String elementValue = reader.getElementText();
+ final String value = elementValue.length() ==
0 && defaultValue.length() != 0 ? defaultValue : elementValue;
Review comment:
After some more thought, it seems that perhaps we might be both right in
our respective thinking on how this should work. There is enough ambiguity to
warrant either approach it seems and if we use Gephi as our model (which
@krlawrence was good enough to test), then we find that they have implemented
it both your way and mine. I think we should follow suit and implement it both
ways as well.
From your perspective, @amatiushkin you may choose to leave your
implementation as it is in this PR and we can look to review/merge where it
stands. If you take this approach, we can just create another JIRA to have it
also work the way I had interpreted the specification. Alternatively, you can
implement both approaches on this pull request to match the functionality as
Gephi has it.
Please let us know how you intend to proceed. Thanks for your patience on
this by the way. I would not have guessed this issue to generate as much
commenting as it has. 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
