[ 
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)

Reply via email to