[ https://issues.apache.org/jira/browse/TINKERPOP-848?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17427123#comment-17427123 ]
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_r726113446 ########## File path: gremlin-core/src/test/resources/graphml/sample.graphml.xml ########## @@ -0,0 +1,367 @@ +<?xml version="1.0" encoding="UTF-8"?> Review comment: I can see the need for a new test file as you are adding a new feature here and I don't really think we should change the existing GraphML files. As a suggestion, it would be nice if the new test file simply was a modification of the existing "modern" data set. We try to keep the datasets consistent when we can even for test. It's not always possible but in this case I think it would be easy to copy the tinkerpop-modern.xml file and modify it to include default values. An easy one would be to include defaults for "lang" and/or "age" which aren't shared across all vertex labels. For edge defaults I suppose you would have to fabricate something as all edges have "weight" but at least the base graph data would be the same. if you have some good reason for sticking with this data as you have though, please let me know, otherwise it would be nice to see our more familiar dataset in use here. -- 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)