jrmccluskey commented on code in PR #31657:
URL: https://github.com/apache/beam/pull/31657#discussion_r1691765629


##########
examples/notebooks/beam-ml/rag_usecase/chunks_generation.py:
##########
@@ -0,0 +1,130 @@
+#
+# 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.
+#
+
+from __future__ import absolute_import
+
+import apache_beam as beam
+from langchain.text_splitter import CharacterTextSplitter, 
RecursiveCharacterTextSplitter
+from langchain_text_splitters import SentenceTransformersTokenTextSplitter
+
+from apache_beam.transforms import DoFn
+from apache_beam.transforms import PTransform
+from enum import Enum
+
+
+__all__ = [
+    'ChunksGeneration',
+    'ChunkingStrategy'
+]
+
+class ChunkingStrategy(Enum):
+    SPLIT_BY_CHARACTER = 0
+    RECURSIVE_SPLIT_BY_CHARACTER = 1
+    SPLIT_BY_TOKENS = 2
+
+
+class ChunksGeneration(PTransform):
+    """ChunkingStrategy is a ``PTransform`` that takes a ``PCollection`` of
+    key, value tuple or 2-element array and generates different chunks for 
documents.
+    """
+
+    def __init__(
+            self,
+            chunk_size: int,
+            chunk_overlap: int,
+            chunking_strategy: str,

Review Comment:
   You can type hint the enum directly so users _have_ to use one of the 
provided options
   
   https://www.geeksforgeeks.org/type-hint-enum-in-python/



##########
examples/notebooks/beam-ml/rag_usecase/redis_connector.py:
##########
@@ -0,0 +1,362 @@
+#
+# 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.
+#
+
+from __future__ import absolute_import
+
+import logging
+from past.builtins import unicode
+
+import apache_beam as beam
+import numpy as np
+
+from apache_beam.transforms import DoFn
+from apache_beam.transforms import PTransform
+from apache_beam.transforms import Reshuffle
+from apache_beam import coders
+
+import redis
+import typing
+from typing import Optional
+
+# Set the logging level to reduce verbose information
+import logging
+
+logging.root.setLevel(logging.INFO)
+logger = logging.getLogger(__name__)
+
+__all__ = ['InsertDocInRedis', 'InsertEmbeddingInRedis']
+
+
+class Document(typing.NamedTuple):
+    id: str
+    url: str
+    title: str
+    text: str
+
+
+coders.registry.register_coder(Document, coders.RowCoder)
+
+"""This module implements IO classes to read write documents in Redis.
+
+
+Insert Doc in Redis:
+-----------------
+:class:`InsertDocInRedis` is a ``PTransform`` that writes key and values to a
+configured sink, and the write is conducted through a redis pipeline.
+
+The ptransform works by getting the first and second elements from the input,
+this means that inputs like `[k,v]` or `(k,v)` are valid.
+
+Example usage::
+
+  pipeline | InsertDocInRedis(host='localhost',
+                          port=6379,
+                          batch_size=100)
+"""
+
+
+class InsertDocInRedis(PTransform):
+    """InsertDocInRedis is a ``PTransform`` that writes a ``PCollection`` of
+    key, value tuple or 2-element array into a redis server.
+    """
+
+    def __init__(self,
+                 host: str,
+                 port: int,
+                 command: Optional[str] = None,
+                 batch_size: int = 100
+                 ):
+
+        """
+
+        Args:
+        host (str): The redis host
+        port (int): The redis port
+        batch_size(int): Number of key, values pairs to write at once
+        command (str): command to be executed with redis client

Review Comment:
   Swap to make sure the args match the order in the function signature



##########
examples/notebooks/beam-ml/rag_usecase/redis_connector.py:
##########
@@ -0,0 +1,362 @@
+#
+# 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.
+#
+
+from __future__ import absolute_import
+
+import logging
+from past.builtins import unicode
+
+import apache_beam as beam
+import numpy as np
+
+from apache_beam.transforms import DoFn
+from apache_beam.transforms import PTransform
+from apache_beam.transforms import Reshuffle
+from apache_beam import coders
+
+import redis
+import typing
+from typing import Optional
+
+# Set the logging level to reduce verbose information
+import logging
+
+logging.root.setLevel(logging.INFO)
+logger = logging.getLogger(__name__)
+
+__all__ = ['InsertDocInRedis', 'InsertEmbeddingInRedis']
+
+
+class Document(typing.NamedTuple):
+    id: str
+    url: str
+    title: str
+    text: str
+
+
+coders.registry.register_coder(Document, coders.RowCoder)
+
+"""This module implements IO classes to read write documents in Redis.
+
+
+Insert Doc in Redis:
+-----------------
+:class:`InsertDocInRedis` is a ``PTransform`` that writes key and values to a
+configured sink, and the write is conducted through a redis pipeline.
+
+The ptransform works by getting the first and second elements from the input,
+this means that inputs like `[k,v]` or `(k,v)` are valid.
+
+Example usage::
+
+  pipeline | InsertDocInRedis(host='localhost',
+                          port=6379,
+                          batch_size=100)
+"""
+
+
+class InsertDocInRedis(PTransform):
+    """InsertDocInRedis is a ``PTransform`` that writes a ``PCollection`` of
+    key, value tuple or 2-element array into a redis server.
+    """
+
+    def __init__(self,
+                 host: str,
+                 port: int,
+                 command: Optional[str] = None,
+                 batch_size: int = 100
+                 ):
+
+        """
+
+        Args:
+        host (str): The redis host
+        port (int): The redis port
+        batch_size(int): Number of key, values pairs to write at once
+        command (str): command to be executed with redis client
+
+        Returns:
+        :class:`~apache_beam.transforms.ptransform.PTransform`
+
+        """
+
+        self._host = host
+        self._port = port
+        self._command = command
+        self._batch_size = batch_size
+
+    def expand(self, pcoll):
+        return pcoll \
+               | "Reshuffle for Redis Insert" >> Reshuffle() \
+               | "Insert document into Redis" >> 
beam.ParDo(_InsertDocRedisFn(self._host,
+                                                                              
self._port,
+                                                                              
self._command,
+                                                                              
self._batch_size)
+                                                            # 
.with_output_types(Document)

Review Comment:
   Remove the comment if you aren't using it



##########
examples/notebooks/beam-ml/rag_usecase/redis_connector.py:
##########
@@ -0,0 +1,362 @@
+#
+# 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.
+#
+
+from __future__ import absolute_import
+
+import logging
+from past.builtins import unicode
+
+import apache_beam as beam
+import numpy as np
+
+from apache_beam.transforms import DoFn
+from apache_beam.transforms import PTransform
+from apache_beam.transforms import Reshuffle
+from apache_beam import coders
+
+import redis
+import typing
+from typing import Optional
+
+# Set the logging level to reduce verbose information
+import logging
+
+logging.root.setLevel(logging.INFO)
+logger = logging.getLogger(__name__)
+
+__all__ = ['InsertDocInRedis', 'InsertEmbeddingInRedis']
+
+
+class Document(typing.NamedTuple):
+    id: str
+    url: str
+    title: str
+    text: str
+
+
+coders.registry.register_coder(Document, coders.RowCoder)
+
+"""This module implements IO classes to read write documents in Redis.
+
+
+Insert Doc in Redis:
+-----------------
+:class:`InsertDocInRedis` is a ``PTransform`` that writes key and values to a
+configured sink, and the write is conducted through a redis pipeline.
+
+The ptransform works by getting the first and second elements from the input,
+this means that inputs like `[k,v]` or `(k,v)` are valid.
+
+Example usage::
+
+  pipeline | InsertDocInRedis(host='localhost',
+                          port=6379,
+                          batch_size=100)
+"""
+
+
+class InsertDocInRedis(PTransform):
+    """InsertDocInRedis is a ``PTransform`` that writes a ``PCollection`` of
+    key, value tuple or 2-element array into a redis server.
+    """
+
+    def __init__(self,
+                 host: str,
+                 port: int,
+                 command: Optional[str] = None,
+                 batch_size: int = 100
+                 ):
+
+        """
+
+        Args:
+        host (str): The redis host
+        port (int): The redis port
+        batch_size(int): Number of key, values pairs to write at once
+        command (str): command to be executed with redis client
+
+        Returns:
+        :class:`~apache_beam.transforms.ptransform.PTransform`
+
+        """
+
+        self._host = host
+        self._port = port
+        self._command = command
+        self._batch_size = batch_size
+
+    def expand(self, pcoll):
+        return pcoll \
+               | "Reshuffle for Redis Insert" >> Reshuffle() \
+               | "Insert document into Redis" >> 
beam.ParDo(_InsertDocRedisFn(self._host,
+                                                                              
self._port,
+                                                                              
self._command,
+                                                                              
self._batch_size)
+                                                            # 
.with_output_types(Document)
+                                                            )
+
+
+class _InsertDocRedisFn(DoFn):
+    """Abstract class that takes in redis
+    credentials to connect to redis DB
+    """
+
+    def __init__(self,
+                 host: str,
+                 port: int,
+                 command: Optional[str] = None,
+                 batch_size: int = 100
+                 ):
+        self.host = host
+        self.port = port
+        self.command = command
+        self.batch_size = batch_size
+
+        self.batch_counter = 0
+        self.batch = list()
+
+        self.text_col = None
+
+    def finish_bundle(self):
+        self._flush()
+
+    def process(self, element, *args, **kwargs):
+        # if type(element) is not dict:
+        #     element = element._asdict()
+        self.batch.append(element)
+        self.batch_counter += 1
+        if self.batch_counter >= self.batch_size:
+            self._flush()
+            # yield Document(**element)

Review Comment:
   Same thing here, if the commented code isn't going to be used now it's 
better to get rid of it and replace it with a TODO for future improvment



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to