Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/892#discussion_r202653620
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
---
@@ -218,20 +220,33 @@ private void writeTypes(final Map<String, String>
identifiedVertexKeyTypes,
final Map<String, String>
identifiedEdgeKeyTypes,
final XMLStreamWriter writer) throws
XMLStreamException {
// <key id="weight" for="edge" attr.name="weight"
attr.type="float"/>
- final Collection<String> vertexKeySet =
getVertexKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
+ Collection<String> vertexKeySet =
getDataStructureKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
+ Collection<String> edgeKeySet =
getDataStructureKeysAndNormalizeIfRequired(identifiedEdgeKeyTypes);
+ // in case vertex and edge may have the same attribute name, the
key id in graphml have to be different
+ intersection = CollectionUtils.intersection(vertexKeySet,
edgeKeySet);
--- End diff --
In one respect, I think that using a suffix in the `id` is a good idea
generally - in other words, don't bother to even track the intersections, just
add the suffix, but I'm not sure how different tools use the `id` attribute. I
assume they use it as we use it, just to look up the `attr.name` and
`attr.value` - if so, then the `id` is simply internal to the graphml and you
don't need to track for intersections to add the suffix.
If that is not the case and the choice is to not generalize this, then we'd
need tests to cover this logic.
---
