[GitHub] tinkerpop pull request #905: Pr 891
Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/905 ---
[GitHub] tinkerpop pull request #905: Pr 891
Github user spmallette commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/905#discussion_r209302881 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java --- @@ -18,6 +18,21 @@ */ package org.apache.tinkerpop.gremlin.structure.io.graphml; +import java.io.IOException; --- End diff -- i just happened to peek back in on this PR and noticed that it looks like your IDE re-organized the imports a bit. when you come back to this could you please revert that portion of the change? ---
[GitHub] tinkerpop pull request #905: Pr 891
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/905#discussion_r207705128
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
---
@@ -212,26 +214,39 @@ private XMLStreamWriter configureWriter(final
OutputStream outputStream) throws
return writer;
} else
return utf8Writer;
-}
+}
private void writeTypes(final Map
identifiedVertexKeyTypes,
-final Map
identifiedEdgeKeyTypes,
-final XMLStreamWriter writer) throws
XMLStreamException {
+final Map identifiedEdgeKeyTypes,
+final XMLStreamWriter writer) throws XMLStreamException {
//
final Collection vertexKeySet =
getVertexKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
+final Collection edgeKeySet =
getEdgeKeysAndNormalizeIfRequired(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);
+// speeding-up later checks
+if (intersection.isEmpty()) {
--- End diff --
i like the suggestion @twilmes had for this here:
https://github.com/apache/tinkerpop/pull/891#pullrequestreview-139083855
it sounds like a more fluent way to handle this logic.
---
[GitHub] tinkerpop pull request #905: Pr 891
Github user spmallette commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/905#discussion_r207241886 --- Diff: gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/tinkerpop-classic-normalized.xml --- @@ -1,15 +1,18 @@ http://graphml.graphdrawing.org/xmlns"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://graphml.graphdrawing.org/xmlns http://graphml.graphdrawing.org/xmlns/1.1/graphml.xsd";> + --- End diff -- I think you'd missed my comment on the JIRA about this - please add a test here: https://github.com/apache/tinkerpop/blob/e3018fb2e98df62a21272446f63cea5ee550408e/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java#L108 then , since it is a one-off test just for GraphML, I think you can just embed a small GraphML file as a String right in the test. ---
[GitHub] tinkerpop pull request #905: Pr 891
Github user svanteschubert commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/905#discussion_r207240077 --- Diff: gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/tinkerpop-classic-normalized.xml --- @@ -1,15 +1,18 @@ http://graphml.graphdrawing.org/xmlns"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://graphml.graphdrawing.org/xmlns http://graphml.graphdrawing.org/xmlns/1.1/graphml.xsd";> + --- End diff -- Well. than perhaps rename it and place it aside? The problem being solved is to add two properties on edge and vertex having the same name during run-time and serialize this new graph to GraphML. ---
[GitHub] tinkerpop pull request #905: Pr 891
Github user spmallette commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/905#discussion_r207239273 --- Diff: gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/tinkerpop-classic-normalized.xml --- @@ -1,15 +1,18 @@ http://graphml.graphdrawing.org/xmlns"; xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; xsi:schemaLocation="http://graphml.graphdrawing.org/xmlns http://graphml.graphdrawing.org/xmlns/1.1/graphml.xsd";> + --- End diff -- This is a standard dataset. We can't change the data within it. ---
[GitHub] tinkerpop pull request #905: Pr 891
GitHub user svanteschubert opened a pull request: https://github.com/apache/tinkerpop/pull/905 Pr 891 I have added a test and adjusted the changed according to the comments. You can merge this pull request into a Git repository by running: $ git pull https://github.com/svanteschubert/tinkerpop pr-891 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/905.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #905 commit cfc7ecefd370ac438afea03f1e6d718f1eff0fbe Author: Svante Schubert Date: 2018-07-15T18:26:10Z TINKERPOP-2006 - Fix for valid GraphML export when graph properties of a vertex and edge have similar name commit 41f2ab8a5cd6bdb60a195489fce38ef38baaf232 Author: Svante Schubert Date: 2018-08-02T13:33:04Z TINKERPOP-2006 Fixing GraphML serialization with similar named property for edge and vertex ---
