You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/07/14 11:04:16 UTC

[GitHub] [flink-ml] zhipeng93 opened a new pull request, #130: [FLINK-28502] Add Transformer for RegexTokenizer

zhipeng93 opened a new pull request, #130:
URL: https://github.com/apache/flink-ml/pull/130

   This PR adds RegexTokenizer to Flink ML.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-ml] yunfengzhou-hub commented on a diff in pull request #130: [FLINK-28502] Add Transformer for RegexTokenizer

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on code in PR #130:
URL: https://github.com/apache/flink-ml/pull/130#discussion_r924009394


##########
flink-ml-lib/src/main/java/org/apache/flink/ml/feature/regextokenizer/RegexTokenizerParams.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.ml.feature.regextokenizer;
+
+import org.apache.flink.ml.common.param.HasInputCol;
+import org.apache.flink.ml.common.param.HasOutputCol;
+import org.apache.flink.ml.param.BooleanParam;
+import org.apache.flink.ml.param.IntParam;
+import org.apache.flink.ml.param.ParamValidators;
+import org.apache.flink.ml.param.StringParam;
+
+/**
+ * Params for {@link RegexTokenizer}.
+ *
+ * @param <T> The class type of this instance.
+ */
+public interface RegexTokenizerParams<T> extends HasInputCol<T>, HasOutputCol<T> {
+    IntParam MIN_TOKEN_LENGTH =
+            new IntParam("minTokenLength", "Minimum token length", 1, ParamValidators.gtEq(0));
+    BooleanParam GAPS = new BooleanParam("gaps", "Set regex to match gaps or tokens", true);
+
+    StringParam PATTERN = new StringParam("pattern", "Regex pattern used for tokenizing", "\\s+");
+
+    BooleanParam TO_LOWERCASE =
+            new BooleanParam(
+                    "toLowercase",
+                    "Whether to convert all characters to lowercase before tokenizing",
+                    true);
+
+    default T setMinTokenLength(int value) {
+        return set(MIN_TOKEN_LENGTH, value);
+    }
+
+    default Integer getMinTokenLength() {

Review Comment:
   we can make this method return `int` instead of `Integer`, as `HasMaxIter` has done, and remove the `intValue()` in tests.



##########
flink-ml-examples/src/main/java/org/apache/flink/ml/examples/feature/RegexTokenizerExample.java:
##########
@@ -0,0 +1,61 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.ml.examples.feature;
+
+import org.apache.flink.ml.feature.regextokenizer.RegexTokenizer;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.streaming.api.environment.StreamExecutionEnvironment;
+import org.apache.flink.table.api.Table;
+import org.apache.flink.table.api.bridge.java.StreamTableEnvironment;
+import org.apache.flink.types.Row;
+import org.apache.flink.util.CloseableIterator;
+
+import java.util.Arrays;
+
+/** Simple program that creates a RegexTokenizer instance and uses it for feature engineering. */
+public class RegexTokenizerExample {
+    public static void main(String[] args) {
+        StreamExecutionEnvironment env = StreamExecutionEnvironment.getExecutionEnvironment();
+        StreamTableEnvironment tEnv = StreamTableEnvironment.create(env);
+
+        // Generates input data.
+        DataStream<Row> inputStream =
+                env.fromElements(Row.of("Test for tokenization."), Row.of("Te,st. punct"));
+        Table inputTable = tEnv.fromDataStream(inputStream).as("input");
+
+        // Creates a RegexTokenizer object and initializes its parameters.
+        RegexTokenizer regexTokenizer =
+                new RegexTokenizer().setInputCol("input").setOutputCol("output");

Review Comment:
   It might be better to add a `setPattern()` here to show the distinguished function of RegexTokenizer from Tokenizer.



##########
flink-ml-lib/src/main/java/org/apache/flink/ml/feature/regextokenizer/RegexTokenizerParams.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.flink.ml.feature.regextokenizer;
+
+import org.apache.flink.ml.common.param.HasInputCol;
+import org.apache.flink.ml.common.param.HasOutputCol;
+import org.apache.flink.ml.param.BooleanParam;
+import org.apache.flink.ml.param.IntParam;
+import org.apache.flink.ml.param.ParamValidators;
+import org.apache.flink.ml.param.StringParam;
+
+/**
+ * Params for {@link RegexTokenizer}.
+ *
+ * @param <T> The class type of this instance.
+ */
+public interface RegexTokenizerParams<T> extends HasInputCol<T>, HasOutputCol<T> {
+    IntParam MIN_TOKEN_LENGTH =
+            new IntParam("minTokenLength", "Minimum token length", 1, ParamValidators.gtEq(0));
+    BooleanParam GAPS = new BooleanParam("gaps", "Set regex to match gaps or tokens", true);

Review Comment:
   nit: adding a blank line above this line might be better.



-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-ml] lindong28 commented on pull request #130: [FLINK-28502] Add Transformer for RegexTokenizer

Posted by GitBox <gi...@apache.org>.
lindong28 commented on PR #130:
URL: https://github.com/apache/flink-ml/pull/130#issuecomment-1190026797

   Thanks for the PR! LGTM.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-ml] yunfengzhou-hub commented on pull request #130: [FLINK-28502] Add Transformer for RegexTokenizer

Posted by GitBox <gi...@apache.org>.
yunfengzhou-hub commented on PR #130:
URL: https://github.com/apache/flink-ml/pull/130#issuecomment-1189696621

   Thanks for the update. It LGTM now.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-ml] zhipeng93 commented on pull request #130: [FLINK-28502] Add Transformer for RegexTokenizer

Posted by GitBox <gi...@apache.org>.
zhipeng93 commented on PR #130:
URL: https://github.com/apache/flink-ml/pull/130#issuecomment-1188889974

   Thanks for the review @yunfengzhou-hub . I have addressed your comments in the latest commit.


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-ml] lindong28 merged pull request #130: [FLINK-28502] Add Transformer for RegexTokenizer

Posted by GitBox <gi...@apache.org>.
lindong28 merged PR #130:
URL: https://github.com/apache/flink-ml/pull/130


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org