spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r730894525
##########
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:
> I need to better understand the intention to move default value
assignment under XMLEvent.END_ELEMENT.
Maybe i'm missing something but as I understand things, if "there is no
default value therefore related events won't be emitted at all" then your code
will only add a default value if the key is present with an empty string. If
that is correct, that leads to two issues: (1) that means user will be
precluded from using empty string as a valid value if they choose to use
defaults. (2) I don't think that this is the intention of the defaults in
GraphML. Section 2.4.3 of the GraphML Primer explains the expected usage that I
tried to describe in my last comment:
http://graphml.graphdrawing.org/primer/graphml-primer.html#AttributesValues
Perhaps their explanation will clarify it for you.
> It would require to make an extra call to typeCastValue(...) for default
values as well before making an assignment.
Is there a reason you couldn't cache these defaults when they are first
parsed at `GraphMLTokens.DEFAULT`? Instead of storing element text, just call
`typeCastValue()` there and then in `END_ELEMENT` detect which keys are not
present and inject the defaults. wdyt?
--
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]
