Rename feature proc + yieldColumn in NC pipelines

FlorentinD · FlorentinD · commit 88d56d1781d6 · 2021-11-30T18:05:44.000+01:00
Needed to extract the featurePipelineDescription.
diff --git a/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/linkmodels/pipeline/LinkPredictionPipeline.java b/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/linkmodels/pipeline/LinkPredictionPipeline.java
@@ -19,6 +19,7 @@
  */
 package org.neo4j.gds.ml.linkmodels.pipeline;
 
+import org.neo4j.gds.config.ToMapConvertible;
 import org.neo4j.gds.ml.linkmodels.pipeline.linkFeatures.LinkFeatureStep;
 import org.neo4j.gds.ml.linkmodels.pipeline.logisticRegression.LinkLogisticRegressionTrainConfig;
 import org.neo4j.gds.ml.pipeline.Pipeline;
@@ -48,6 +49,15 @@ public LinkPredictionPipeline copy() {
         return copied;
     }
 
+
+    @Override
+    protected Map<String, List<Map<String, Object>>> featurePipelineDescription() {
+        return Map.of(
+            "nodePropertySteps", ToMapConvertible.toMap(nodePropertySteps),
+            "featureSteps", ToMapConvertible.toMap(featureSteps)
+        );
+    }
+
     @Override
     protected Map<String, Object> additionalEntries() {
         return Map.of(
diff --git a/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipeline.java b/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipeline.java
@@ -19,6 +19,7 @@
  */
 package org.neo4j.gds.ml.nodemodels.pipeline;
 
+import org.neo4j.gds.config.ToMapConvertible;
 import org.neo4j.gds.ml.nodemodels.logisticregression.NodeLogisticRegressionTrainCoreConfig;
 import org.neo4j.gds.ml.pipeline.Pipeline;
 
@@ -44,6 +45,15 @@ public NodeClassificationPipeline copy() {
         return copied;
     }
 
+
+    @Override
+    protected Map<String, List<Map<String, Object>>> featurePipelineDescription() {
+        return Map.of(
+            "nodePropertySteps", ToMapConvertible.toMap(nodePropertySteps),
+            "featureProperties", ToMapConvertible.toMap(featureSteps)
+        );
+    }
+
     @Override
     protected Map<String, Object> additionalEntries() {
         return Map.of(
diff --git a/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddStepProcs.java b/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddStepProcs.java
@@ -53,12 +53,12 @@ public Stream<PipelineInfoResult> addNodeProperty(
         ));
     }
 
-    @Procedure(name = "gds.alpha.ml.pipeline.nodeClassification.addFeatures", mode = READ)
+    @Procedure(name = "gds.alpha.ml.pipeline.nodeClassification.selectFeatures", mode = READ)
     @Description("Add one or several features to an existing node classification pipeline.")
-    public Stream<PipelineInfoResult> addFeatures(
+    public Stream<PipelineInfoResult> selectFeatures(
         @Name("pipelineName") String pipelineName,
         @Name("nodeProperties") Object nodeProperties
     ) {
-        return Stream.of(NodeClassificationPipelineAddSteps.addFeatures(modelCatalog, username(), pipelineName, nodeProperties));
+        return Stream.of(NodeClassificationPipelineAddSteps.selectFeatures(modelCatalog, username(), pipelineName, nodeProperties));
     }
 }
diff --git a/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddSteps.java b/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddSteps.java
@@ -44,7 +44,7 @@ public static PipelineInfoResult addNodeProperty(
         return new PipelineInfoResult(pipelineName, pipeline);
     }
 
-    public static PipelineInfoResult addFeatures(
+    public static PipelineInfoResult selectFeatures(
         ModelCatalog modelCatalog,
         String username,
         String pipelineName,
diff --git a/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/PipelineInfoResult.java b/alpha/alpha-proc/src/main/java/org/neo4j/gds/ml/nodemodels/pipeline/PipelineInfoResult.java
@@ -29,7 +29,7 @@
 public class PipelineInfoResult {
     public final String name;
     public final List<Map<String, Object>> nodePropertySteps;
-    public final List<String> featureSteps;
+    public final List<String> featureProperties;
     public final Map<String, Object> splitConfig;
     public final Object parameterSpace;
 
@@ -40,7 +40,7 @@ public class PipelineInfoResult {
             .stream()
             .map(ExecutableNodePropertyStep::toMap)
             .collect(Collectors.toList());
-        this.featureSteps = info.featureProperties();
+        this.featureProperties = info.featureProperties();
         this.splitConfig = info.splitConfig().toMap();
         this.parameterSpace = info.trainingParameterSpace()
             .stream()
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddStepsTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineAddStepsTest.java
@@ -77,21 +77,21 @@ void shouldAddNodePropertyStep() {
                     "name", "gds.pageRank.mutate",
                     "config", Map.of("mutateProperty", "pr")
                 )));
-            assertThat(result.featureSteps).isEqualTo(List.of());
+            assertThat(result.featureProperties).isEqualTo(List.of());
             assertThat(result.parameterSpace).isEqualTo(DEFAULT_PARAM_CONFIG);
         });
     }
 
     @Test
-    void shouldAddFeatureSteps() {
+    void shouldSelectFeatures() {
         run(caller -> {
-            NodeClassificationPipelineAddSteps.addFeatures(
+            NodeClassificationPipelineAddSteps.selectFeatures(
                 modelCatalog,
                 getUsername(),
                 "myPipeline",
                 "test"
             );
-            var result = NodeClassificationPipelineAddSteps.addFeatures(
+            var result = NodeClassificationPipelineAddSteps.selectFeatures(
                 modelCatalog,
                 getUsername(),
                 "myPipeline",
@@ -100,7 +100,7 @@ void shouldAddFeatureSteps() {
             assertThat(result.name).isEqualTo("myPipeline");
             assertThat(result.splitConfig).isEqualTo(DEFAULT_CONFIG.toMap());
             assertThat(result.nodePropertySteps).isEqualTo(List.of());
-            assertThat(result.featureSteps).isEqualTo(List.of("test", "pr", "pr2"));
+            assertThat(result.featureProperties).isEqualTo(List.of("test", "pr", "pr2"));
             assertThat(result.parameterSpace).isEqualTo(DEFAULT_PARAM_CONFIG);
         });
     }
@@ -161,7 +161,7 @@ void failOnUnexpectedConfigKeysInNodePropertyStepConfig() {
     }
 
     @Test
-    void shouldAddNodeAndFeatureSteps() {
+    void shouldAddNodeAndSelectFeatureProperties() {
         run(caller -> {
             NodeClassificationPipelineAddSteps.addNodeProperty(
                 modelCatalog,
@@ -171,13 +171,13 @@ void shouldAddNodeAndFeatureSteps() {
                 "pageRank",
                 Map.of("mutateProperty", "pr")
             );
-            NodeClassificationPipelineAddSteps.addFeatures(
+            NodeClassificationPipelineAddSteps.selectFeatures(
                 modelCatalog,
                 getUsername(),
                 "myPipeline",
                 "pr"
             );
-            var result = NodeClassificationPipelineAddSteps.addFeatures(
+            var result = NodeClassificationPipelineAddSteps.selectFeatures(
                 modelCatalog,
                 getUsername(),
                 "myPipeline",
@@ -190,7 +190,7 @@ void shouldAddNodeAndFeatureSteps() {
                     "name", "gds.pageRank.mutate",
                     "config", Map.of("mutateProperty", "pr")
                 )));
-            assertThat(result.featureSteps).isEqualTo(List.of("pr", "pr2"));
+            assertThat(result.featureProperties).isEqualTo(List.of("pr", "pr2"));
             assertThat(result.parameterSpace).isEqualTo(DEFAULT_PARAM_CONFIG);
         });
     }
@@ -226,7 +226,7 @@ void shouldAddTwoNodePropertySteps() {
                     "config", Map.of("mutateProperty", "pr2")
                 ))
             );
-            assertThat(result.featureSteps).isEqualTo(List.of());
+            assertThat(result.featureProperties).isEqualTo(List.of());
             assertThat(result.parameterSpace).isEqualTo(DEFAULT_PARAM_CONFIG);
         });
     }
@@ -247,7 +247,7 @@ void shouldThrowIfPipelineDoesntExistForNodePropertyStep() {
 
     @Test
     void shouldThrowIfPipelineDoesntExistForFeatureStep() {
-        assertThatThrownBy(() -> NodeClassificationPipelineAddSteps.addFeatures(
+        assertThatThrownBy(() -> NodeClassificationPipelineAddSteps.selectFeatures(
             modelCatalog,
             getUsername(),
             "ceci n'est pas une pipe",
@@ -327,7 +327,7 @@ void shouldThrowIfAddingFeatureToANonPipeline() {
         );
 
         modelCatalog.set(model1);
-        run(caller -> assertThatThrownBy(() -> NodeClassificationPipelineAddSteps.addFeatures(
+        run(caller -> assertThatThrownBy(() -> NodeClassificationPipelineAddSteps.selectFeatures(
             modelCatalog,
             getUsername(),
             "testModel1",
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineConfigureParamsProcTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineConfigureParamsProcTest.java
@@ -53,7 +53,7 @@ void shouldSetParams() {
                 "name", "myPipeline",
                 "splitConfig", DEFAULT_SPLIT_CONFIG,
                 "nodePropertySteps", List.of(),
-                "featureSteps", List.of(),
+                "featureProperties", List.of(),
                 "parameterSpace", List.of(Map.of(
                     "maxEpochs", 100,
                     "minEpochs", 42,
@@ -76,7 +76,7 @@ void shouldOnlyKeepLastOverride() {
                 "myPipeline",
                 "splitConfig", DEFAULT_SPLIT_CONFIG,
                 "nodePropertySteps", List.of(),
-                "featureSteps", List.of(),
+                "featureProperties", List.of(),
                 "parameterSpace", List.of(Map.of(
                     "maxEpochs", 100,
                     "minEpochs", 4,
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineConfigureSplitProcTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineConfigureSplitProcTest.java
@@ -56,7 +56,7 @@ void shouldOverrideSingleSplitField() {
                 "name", "myPipeline",
                 "splitConfig", expectedSplitConfig,
                 "nodePropertySteps", List.of(),
-                "featureSteps", List.of(),
+                "featureProperties", List.of(),
                 "parameterSpace", DEFAULT_PARAM_CONFIG
             ))
         );
@@ -75,7 +75,7 @@ void shouldOnlyKeepLastOverride() {
                 "name", "myPipeline",
                 "splitConfig", expectedSplitConfig,
                 "nodePropertySteps", List.of(),
-                "featureSteps", List.of(),
+                "featureProperties", List.of(),
                 "parameterSpace", DEFAULT_PARAM_CONFIG
             ))
         );
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineCreateTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineCreateTest.java
@@ -57,7 +57,7 @@ void createPipeline() {
         var result = NodeClassificationPipelineCreate.create(modelCatalog, getUsername(), "myPipeline");
         assertThat(result.name).isEqualTo("myPipeline");
         assertThat(result.nodePropertySteps).isEqualTo(List.of());
-        assertThat(result.featureSteps).isEqualTo(List.of());
+        assertThat(result.featureProperties).isEqualTo(List.of());
         assertThat(result.splitConfig).isEqualTo(NodeClassificationPipelineCompanion.DEFAULT_SPLIT_CONFIG);
         assertThat(result.parameterSpace).isEqualTo(DEFAULT_PARAM_CONFIG);
 
@@ -70,7 +70,7 @@ void createPipeline() {
                         "modelType", PIPELINE_MODEL_TYPE,
                         "featurePipeline", Map.of(
                             "nodePropertySteps", List.of(),
-                            "featureSteps", List.of()
+                            "featureProperties", List.of()
                         ),
                         "splitConfig", NodeClassificationPipelineCompanion.DEFAULT_SPLIT_CONFIG,
                         "trainingParameterSpace", DEFAULT_PARAM_CONFIG
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineIntegrationTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineIntegrationTest.java
@@ -120,8 +120,8 @@ void trainWithNodePropertyStepsAndFeatures() {
                  "  relationshipWeightProperty: 'w'" +
                  "})");
         // let's try both list and single string syntaxes
-        runQuery("CALL gds.alpha.ml.pipeline.nodeClassification.addFeatures('p', 'a')");
-        runQuery("CALL gds.alpha.ml.pipeline.nodeClassification.addFeatures('p', ['b', 'deg'])");
+        runQuery("CALL gds.alpha.ml.pipeline.nodeClassification.selectFeatures('p', 'a')");
+        runQuery("CALL gds.alpha.ml.pipeline.nodeClassification.selectFeatures('p', ['b', 'deg'])");
 
         runQuery("CALL gds.alpha.ml.pipeline.nodeClassification.configureSplit('p', {" +
                  "  testFraction: 0.2, " +
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/NodeClassificationPipelineTest.java
@@ -48,7 +48,7 @@ void canCreateEmptyPipeline() {
     }
 
     @Test
-    void canAddFeatureSteps() {
+    void canSelectFeature() {
         var pipeline = new NodeClassificationPipeline();
         var fooStep = new NodeClassificationFeatureStep("foo");
         pipeline.addFeatureStep(fooStep);
@@ -144,9 +144,9 @@ void returnsCorrectDefaultsMap() {
                 .satisfies(pipelineMap -> assertThat(pipelineMap.get("featurePipeline"))
                     .isInstanceOf(Map.class)
                     .asInstanceOf(InstanceOfAssertFactories.MAP)
-                    .containsOnlyKeys("nodePropertySteps", "featureSteps")
+                    .containsOnlyKeys("nodePropertySteps", "featureProperties")
                     .returns(List.of(), featurePipelineMap -> featurePipelineMap.get("nodePropertySteps"))
-                    .returns(List.of(), featurePipelineMap -> featurePipelineMap.get("featureSteps")))
+                    .returns(List.of(), featurePipelineMap -> featurePipelineMap.get("featureProperties")))
                 .returns(
                     NodeClassificationSplitConfig.DEFAULT_CONFIG.toMap(),
                     pipelineMap -> pipelineMap.get("splitConfig")
@@ -179,14 +179,14 @@ void returnsCorrectMapWithFullConfiguration() {
                 .satisfies(pipelineMap -> assertThat(pipelineMap.get("featurePipeline"))
                     .isInstanceOf(Map.class)
                     .asInstanceOf(InstanceOfAssertFactories.MAP)
-                    .containsOnlyKeys("nodePropertySteps", "featureSteps")
+                    .containsOnlyKeys("nodePropertySteps", "featureProperties")
                     .returns(
                         List.of(pageRankPropertyStep.toMap()),
                         featurePipelineMap -> featurePipelineMap.get("nodePropertySteps")
                     )
                     .returns(
                         List.of(fooStep.toMap()),
-                        featurePipelineMap -> featurePipelineMap.get("featureSteps")
+                        featurePipelineMap -> featurePipelineMap.get("featureProperties")
                     ))
                 .returns(
                     pipeline.splitConfig().toMap(),
diff --git a/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/predict/NodeClassificationPipelineTrainProcTest.java b/alpha/alpha-proc/src/test/java/org/neo4j/gds/ml/nodemodels/pipeline/predict/NodeClassificationPipelineTrainProcTest.java
@@ -116,7 +116,7 @@ void train() {
             pipe
         );
         runQuery(
-            "CALL gds.alpha.ml.pipeline.nodeClassification.addFeatures($pipeline, ['array', 'scalar', 'pr'])",
+            "CALL gds.alpha.ml.pipeline.nodeClassification.selectFeatures($pipeline, ['array', 'scalar', 'pr'])",
             pipe
         );
         runQuery(
@@ -170,7 +170,7 @@ void train() {
                 .containsEntry("config", Map.of("mutateProperty", "pr"));
 
             featurePipeline
-                .extractingByKey("featureSteps", InstanceOfAssertFactories.list(Map.class))
+                .extractingByKey("featureProperties", InstanceOfAssertFactories.list(Map.class))
                 .extracting(map -> map.get("feature"))
                 .containsExactly("array", "scalar", "pr");
 
diff --git a/doc/asciidoc/algorithms/alpha/nodeclassification-pipeline/nodeclassification.adoc b/doc/asciidoc/algorithms/alpha/nodeclassification-pipeline/nodeclassification.adoc
diff --git a/doc/asciidoc/algorithms/alpha/nodeclassification-pipeline/pipelineInfoResult.adoc b/doc/asciidoc/algorithms/alpha/nodeclassification-pipeline/pipelineInfoResult.adoc
diff --git a/doc/asciidoc/operations-reference/appendix-a-graph-algos.adoc b/doc/asciidoc/operations-reference/appendix-a-graph-algos.adoc
diff --git a/pipeline/src/main/java/org/neo4j/gds/ml/pipeline/Pipeline.java b/pipeline/src/main/java/org/neo4j/gds/ml/pipeline/Pipeline.java
diff --git a/pipeline/src/test/java/org/neo4j/gds/ml/pipeline/PipelineExecutorTest.java b/pipeline/src/test/java/org/neo4j/gds/ml/pipeline/PipelineExecutorTest.java
diff --git a/proc/src/test/java/org/neo4j/gds/ListProcTest.java b/proc/src/test/java/org/neo4j/gds/ListProcTest.java

Original file line number	Diff line number	Diff line change
`@@ -53,12 +53,12 @@ public Stream<PipelineInfoResult> addNodeProperty(`
`53`	`53`	`));`
`54`	`54`	`}`
`55`	`55`
`56`		`- @Procedure(name = "gds.alpha.ml.pipeline.nodeClassification.addFeatures", mode = READ)`
	`56`	`+ @Procedure(name = "gds.alpha.ml.pipeline.nodeClassification.selectFeatures", mode = READ)`
`57`	`57`	`@Description("Add one or several features to an existing node classification pipeline.")`
`58`		`- public Stream<PipelineInfoResult> addFeatures(`
	`58`	`+ public Stream<PipelineInfoResult> selectFeatures(`
`59`	`59`	`@Name("pipelineName") String pipelineName,`
`60`	`60`	`@Name("nodeProperties") Object nodeProperties`
`61`	`61`	`) {`
`62`		`- return Stream.of(NodeClassificationPipelineAddSteps.addFeatures(modelCatalog, username(), pipelineName, nodeProperties));`
	`62`	`+ return Stream.of(NodeClassificationPipelineAddSteps.selectFeatures(modelCatalog, username(), pipelineName, nodeProperties));`
`63`	`63`	`}`
`64`	`64`	`}`
Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,7 @@ public static PipelineInfoResult addNodeProperty(`
`44`	`44`	`return new PipelineInfoResult(pipelineName, pipeline);`
`45`	`45`	`}`
`46`	`46`
`47`		`- public static PipelineInfoResult addFeatures(`
	`47`	`+ public static PipelineInfoResult selectFeatures(`
`48`	`48`	`ModelCatalog modelCatalog,`
`49`	`49`	`String username,`
`50`	`50`	`String pipelineName,`