[GitHub] tinkerpop pull request #705: make TinkerGraph cloneable

2017-09-04 Thread mpollmeier
GitHub user mpollmeier opened a pull request:

https://github.com/apache/tinkerpop/pull/705

make TinkerGraph cloneable

most people use tinkergraph for testing, and if there are traversals that 
manipulate the graph, it's useful to be able to clone it up front, especially 
for larger graphs. 

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/mpollmeier/tinkerpop mp/tinkergraph-clone

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/705.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 #705


commit b5c3c1171073d27ed1cda8b77a719b946aa4faaf
Author: Michael Pollmeier 
Date:   2017-09-04T08:02:26Z

make TinkerGraph cloneable




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] tinkerpop issue #705: make TinkerGraph cloneable

2017-09-04 Thread robertdale
Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/705
  
This doesn't look like a deep copy e.g. modifying a property would be 
reflected in both graphs.  Which may be fine. I think any limitations should be 
noted in the clone method comment.


---


[GitHub] tinkerpop issue #705: make TinkerGraph cloneable

2017-09-04 Thread mpollmeier
Github user mpollmeier commented on the issue:

https://github.com/apache/tinkerpop/pull/705
  
That's very true. I just experimented with deep clones and it doesn't look 
straightforward
* kryo can do it directly, but all referenced classes need a 
no-arg-constructor, and since there are some coming in from apache commons, it 
doesn't look like it's feasible. 
https://github.com/EsotericSoftware/kryo#copyingcloning
* java's builtin ByteArrayOutputStream has a similar problem: all 
referenced classes need to be marked as `Serializable`
* implementing `clone` for TinkerVertex, TinkerEdge, TinkerProperty etc. is 
hard because they all reference each other, i.e. you're in a chicken and egg 
situation. 

I guess we have two options: 
1) accept that it's a shallow copy and add a comment, as you suggested
2) disregard this PR

Thoughts? Any other ideas for making a deep copy? There's always the option 
to serialise it into graphml/graphson etc., but that's slow for large graphs. 


---