[
https://issues.apache.org/jira/browse/TINKERPOP-848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17427118#comment-17427118
]
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_r726101651
##########
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 don't recall how `getElementText()` works. If you have `<data
key="d_n"/>` does that return an empty string or `null`? If it's the former,
which your code seems to indicate, doesn't this mean that GraphML can't have
empty strings as values anymore (which may be what the user wants)? I think the
point of the default specification is to deal with a situation like:
```xml
<key id="d_n" for="node" attr.name="modification" attr.type="string">
<default>add</default>
</key>
...
<node id="6315"></node>
```
where the "modification" is not present at all for the `<node><data>` in
which case it would include the default property of "modification" with an
"add" value. otoh, if it were the following:
```xml
<key id="d_n" for="node" attr.name="modification" attr.type="string">
<default>add</default>
</key>
...
<node id="6315">
<data key="modification"/>
</node>
```
it would just include the "modification" property with an empty string. Does
that make sense?
--
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)
