Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1411] added methods TemporalGraph.fromGraph(...) with TimeExtractors #1432

Merged
merged 20 commits into from Feb 10, 2020

Conversation

@xnqio
Copy link
Contributor

xnqio commented Oct 24, 2019

  • added a method TemporalGraph.fromGraph(...) which returns a conversion of the Graph into a Temporal Graph with validTimes depending on the 3 given TimeExtractor Methods
  • added a method TemporalGraphCollection.fromGraphCollection(...) which does the same for GraphCollections
  • created a csv version of the social_network_temporal.gdl file and saved it in gradoop/gradoop-temporal/src/test/resources/datacsv/ for testing purposes
  • fromGraphCollection gets tested in TemporalGraphCollection.testFromGraphCollectionWithTimeExtractors() by comparing the csv files of the social media TemporalGraphCollection with a conversion of the socialmedia graph as a LogicalGraphCollection with the fromGraphCollection(...) function
  • fromGraph(...) gets tested in TemporalGraphTest.testFromGraphWithTimeIntervalExtractors() in the same way as the graph collection, but uses a reduce function to transfer both graphCollections into logical/temporalGraphs
…ollection.fromGraphCollection(...) with TimeExtractor methods as parameters
@xnqio xnqio changed the title [#1411] added methods TemporalGraph.fromLogical(...) / TemporalGraphC… [#1411] added methods TemporalGraph.fromLogical with TimeExtractors Oct 24, 2019
--author= and others added 3 commits Oct 24, 2019
Copy link
Collaborator

p-f left a comment

You do not have to call the time extractor functions manually, you can use the fromNonTemporalDataSets methods from the temporal factories.

Also those methods rely on EPGM classes, it may make sense to generify them to accept BaseGraph and BaseGraphCollection instances instead.

DataSet<TemporalGraphHead> temporalGraphHeadDataSet = logicalGraph.getGraphHead().map(
new GraphHeadToTemporalGraphHead<>(
temporalGradoopConfig.getTemporalGraphFactory().getGraphHeadFactory(), graphTimeExtractor));

DataSet<TemporalVertex> temporalVertexDataSet = logicalGraph.getVertices().map(
new VertexToTemporalVertex<>(temporalGradoopConfig.getTemporalGraphFactory().getVertexFactory(),
vertexTimeExtractor));

DataSet<TemporalEdge> temporalEdgeDataSet = logicalGraph.getEdges().map(
new EdgeToTemporalEdge<>(temporalGradoopConfig.getTemporalGraphFactory().getEdgeFactory(),
edgeTimeExtractor));

return temporalGradoopConfig.getTemporalGraphFactory()
.fromDataSets(temporalGraphHeadDataSet, temporalVertexDataSet, temporalEdgeDataSet);
Comment on lines 254 to 267

This comment has been minimized.

Copy link
@p-f

p-f Oct 24, 2019

Collaborator

You don't have have to do this manually. You can use the TemporalGraphFactory from the TemporalGradoopConfig directly using the fromNonTemporalDataSets(...) method.

temporalGradoopConfig.getTemporalGraphFactory()
  .fromNonTemporalDataSets(
    logicalGraph.getGraphHead(), graphTimeExtractor,
    ...
DataSet<TemporalGraphHead> temporalGraphHeadsDataSet = graphCollection.getGraphHeads().map(
new GraphHeadToTemporalGraphHead<>(
temporalGradoopConfig.getTemporalGraphFactory().getGraphHeadFactory(), graphTimeExtractor));

DataSet<TemporalVertex> temporalVertexDataSet = graphCollection.getVertices().map(
new VertexToTemporalVertex<>(temporalGradoopConfig.getTemporalGraphFactory().getVertexFactory(),
vertexTimeExtractor));

DataSet<TemporalEdge> temporalEdgeDataSet = graphCollection.getEdges().map(
new EdgeToTemporalEdge<>(temporalGradoopConfig.getTemporalGraphFactory().getEdgeFactory(),
edgeTimeExtractor));

return temporalGradoopConfig.getTemporalGraphCollectionFactory()
.fromDataSets(temporalGraphHeadsDataSet, temporalVertexDataSet, temporalEdgeDataSet);
Comment on lines 265 to 278

This comment has been minimized.

Copy link
@p-f

p-f Oct 24, 2019

Collaborator

Same here, the TemporalGraphCollectionFactory has a method called fromNonTemporalDataSets too, which can be used in a similar way.

@Test
public void testFromLogicalGraphWithTimeIntervalExtractors() throws Exception {

String path = TemporalGraphTest.class.getResource("/datacsv/").getFile();

This comment has been minimized.

Copy link
@p-f

p-f Oct 24, 2019

Collaborator

I like this approach, I would just give the file a better name, maybe put them under /data/csv/socialnetwork

/**
* Function to create a {@link TemporalGraph} from an existing {@link LogicalGraph} with valid times
* depending on
* the 3 given {@link TimeIntervalExtractor}

This comment has been minimized.

Copy link
@ChrizZz110

ChrizZz110 Nov 18, 2019

Contributor

Also one line up

Suggested change
* the 3 given {@link TimeIntervalExtractor}
* the three given {@link TimeIntervalExtractor} instances.
* @see TemporalGraphCollectionFactory#fromNonTemporalGraphCollection(BaseGraphCollection)
*/
public static TemporalGraphCollection fromGraphCollection(GraphCollection graphCollection) {
return TemporalGradoopConfig.fromGradoopFlinkConfig(graphCollection.getConfig())
.getTemporalGraphCollectionFactory().fromNonTemporalGraphCollection(graphCollection);
}

/**
* Convenience API function to create a {@link TemporalGraphCollection} from an existing
* {@link GraphCollection} with valid times depending on the 3 given {@link TimeIntervalExtractor} functions

This comment has been minimized.

Copy link
@ChrizZz110

ChrizZz110 Nov 18, 2019

Contributor
Suggested change
* {@link GraphCollection} with valid times depending on the 3 given {@link TimeIntervalExtractor} functions
* {@link GraphCollection} with valid times depending on the three given {@link TimeIntervalExtractor} instances.

Maybe a line break if it exceeds the max chars per line

galpha and others added 5 commits Nov 18, 2019
… of the TemporalGraphFactory().fromNonTemporalDataSet(...) methods
…nto 1411_fromLogical
@xnqio xnqio changed the title [#1411] added methods TemporalGraph.fromLogical with TimeExtractors [#1411] added methods TemporalGraph.fromGraph(...) with TimeExtractors Nov 22, 2019
xnqio and others added 2 commits Nov 25, 2019
Copy link
Contributor

ChrizZz110 left a comment

LGTM

ChrizZz110 and others added 2 commits Nov 26, 2019
@p-f
p-f approved these changes Nov 27, 2019
xnqio and others added 5 commits Dec 9, 2019
…nto 1411_fromLogical
@galpha

This comment has been minimized.

Copy link
Contributor

galpha commented Jan 21, 2020

As soon the merge conflict is fixed I can proceed and merge this PR. @ChrizZz110 could you have a look?

# Conflicts:
#	gradoop-temporal/src/test/java/org/gradoop/temporal/model/impl/TemporalGraphTest.java
@galpha
galpha approved these changes Feb 10, 2020
@galpha galpha merged commit 27b12fa into dbs-leipzig:develop Feb 10, 2020
2 checks passed
2 checks passed
build (ubuntu-latest)
Details
build (macos-latest)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.