[
https://issues.apache.org/jira/browse/TINKERPOP-848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17430000#comment-17430000
]
ASF GitHub Bot commented on TINKERPOP-848:
------------------------------------------
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: commits-unsubscr...@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Support default attribute values in GraphMLReader
> -------------------------------------------------
>
> Key: TINKERPOP-848
> URL: https://issues.apache.org/jira/browse/TINKERPOP-848
> Project: TinkerPop
> Issue Type: Improvement
> Components: io
> Affects Versions: 3.0.2-incubating
> Reporter: Pavel Klinov
> Priority: Trivial
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> Looking at the code of GraphMLReader I see that it doesn't support default
> values of attributes, which are allowed by the GraphML spec. This is a bit
> annoying especially if the input defines default values for attributes which
> are used for mandatory data, e.g. edge labels.
> One small example is the sample graph at [1]. "d_e" is the label attribute
> with a default value. There're <edge .. /> elements w/o body later in the
> document and reading those will throw a "java.lang.IllegalArgumentException:
> Label can not be null" exception (if the vendor considers edge labels
> mandatory).
> I'd personaly squash both keyIdMap and keyTypesMap into a single String ->
> AttrInfo map, where AttrInfo would contain information about the data
> attribute name, type, and the default value.
> [1] http://www.eecs.wsu.edu/~yyao/DirectedStudyI/Datasets/AS/sample.graphml
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
