You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "vijay-x-Raj (via GitHub)" <gi...@apache.org> on 2025/08/15 20:10:27 UTC

[PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

vijay-x-Raj opened a new pull request, #35884:
URL: https://github.com/apache/beam/pull/35884

   The anomaly detection module previously failed when a `[beam.Row]` contained vector features (e.g., embeddings).
   This PR updates the batching logic in `[run_inference]` to flatten both scalar and vector fields, enabling support for use cases like `[beam.Row(embedding=[...])]`.
   No breaking changes; scalar-only rows continue to work as before.
   
   Issue: #35841
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "vijay-x-Raj (via GitHub)" <gi...@apache.org>.
vijay-x-Raj commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3198331505

   There were issues in the original file which caused the tests to fail i have overlapped the file and re wrote codes.
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2280741406


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -75,9 +75,14 @@ def run_inference(
       model: PyODBaseDetector,
       inference_args: Optional[dict[str, Any]] = None
   ) -> Iterable[PredictionResult]:
-    np_batch = []
-    for row in batch:
-      np_batch.append(np.fromiter(row, dtype=np.float64))
+    def _flatten_row(row_values):
+      for value in row_values:
+        if isinstance(value, (list, tuple, np.ndarray)):
+          yield from value
+        else:
+          yield value

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The `_flatten_row` helper function is defined inside `run_inference`, which means it gets redefined on every call. While this is functionally correct, for better performance and readability, consider moving it to the module level (as a private function like `_flatten_row`) or making it a static method on the `PyODModelHandler` class. This would also make it easier to add a docstring and type hints for better maintainability.



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "charlespnh (via GitHub)" <gi...@apache.org>.
charlespnh commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3192816017

   Thanks! I'll defer to @shunping whether Beam's anomaly detection intends to support this use case.


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3234179689

   Fair amount of those linting issues (as in the CI) can be fixed by enforcing proper indentation in the root of your beam project. Recently editor config added to beam (https://github.com/apache/beam/pull/35956). This can be enforced if you are on Visual Studio Code by installing this extension: https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig
   ```
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:61:0: C0301: Line too long (83/80) (line-too-long)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:64:0: C0301: Line too long (83/80) (line-too-long)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:62:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:73:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:74:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:75:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:77:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:78:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:79:0: W0311: Bad indentation. Found 12 spaces, expected 6 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:81:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:87:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:93:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:94:0: W0311: Bad indentation. Found 12 spaces, expected 6 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:95:0: W0311: Bad indentation. Found 16 spaces, expected 8 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:96:0: W0311: Bad indentation. Found 20 spaces, expected 10 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:97:0: W0311: Bad indentation. Found 16 spaces, expected 8 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:98:0: W0311: Bad indentation. Found 20 spaces, expected 10 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:100:0: C0301: Line too long (86/80) (line-too-long)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:100:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:101:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:102:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:103:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:107:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:109:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:110:0: W0311: Bad indentation. Found 4 spaces, expected 2 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:111:0: C0301: Line too long (95/80) (line-too-long)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:111:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:113:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:114:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:115:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:116:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
   ```


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2285131133


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -86,24 +110,36 @@ def run_inference(
     return _convert_to_result(batch, predictions, model_id=self._model_uri)
 
 
-class PyODFactory():
+class PyODFactory:
+  """Factory helpers for creating OfflineDetector instances from PyOD models.
+
+    The factory currently only exposes a convenience method to wrap a pickled
+    PyOD model into an OfflineDetector with a fixed threshold taken from the
+    trained model.
+    """
   @staticmethod
   def create_detector(model_uri: str, **kwargs) -> OfflineDetector:
     """A utility function to create OfflineDetector for a PyOD model.
 
-    **NOTE:** This API and its implementation are currently under active
-    development and may not be backward compatible.
+        **NOTE:** This API and its implementation are currently under active
+        development and may not be backward compatible.
+
+        Args:
+            model_uri: The URI specifying the location of the pickled
+                PyOD model.
+            **kwargs: Additional keyword arguments.
+        """
+    model_handler = (
+        KeyedModelHandler(
+            PyODModelHandler(model_uri=model_uri)).with_postprocess_fn(
+                OfflineDetector.score_prediction_adapter))
 
-    Args:
-      model_uri: The URI specifying the location of the pickled PyOD model.
-      **kwargs: Additional keyword arguments.
-    """
-    model_handler = KeyedModelHandler(
-        PyODModelHandler(model_uri=model_uri)).with_postprocess_fn(
-            OfflineDetector.score_prediction_adapter)
     m = model_handler.load_model()
-    assert (isinstance(m, PyODBaseDetector))
+    assert isinstance(m, PyODBaseDetector)
     threshold = float(m.threshold_)
+
     detector = OfflineDetector(
-        model_handler, threshold_criterion=FixedThreshold(threshold), **kwargs)  # type: ignore[arg-type]
+        model_handler, threshold_criterion=FixedThreshold(threshold), **kwargs

Review Comment:
   Looks like the change here is causing linter error.



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "vijay-x-Raj (via GitHub)" <gi...@apache.org>.
vijay-x-Raj commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3211323974

   yea I will try that and after will keep trying !.
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2292488982


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -29,6 +35,7 @@
 from apache_beam.ml.anomaly.specifiable import specifiable
 from apache_beam.ml.anomaly.thresholds import FixedThreshold
 from apache_beam.ml.inference.base import KeyedModelHandler
+from typing import cast, Any

Review Comment:
   Please put this line after "from typing import Optional" and remove "Any". It has been imported in line 27.
   
   Imports needs to be placed in order and system imports come first, then third-party imports and apache beam imports.



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3485830679

   This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3197269746

   Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment `assign set of reviewers`


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3193561268

   /gemini review


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "vijay-x-Raj (via GitHub)" <gi...@apache.org>.
vijay-x-Raj commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3194166841

   /gemini review
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2282671738


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -75,9 +75,14 @@ def run_inference(
       model: PyODBaseDetector,
       inference_args: Optional[dict[str, Any]] = None
   ) -> Iterable[PredictionResult]:
-    np_batch = []
-    for row in batch:
-      np_batch.append(np.fromiter(row, dtype=np.float64))
+    def _flatten_row(row_values):
+      for value in row_values:
+        if isinstance(value, (list, tuple, np.ndarray)):
+          yield from value
+        else:
+          yield value
+
+    np_batch = [np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch]

Review Comment:
   This line is too long and causing tests on linter and formatter failed. 
   
   Could you make the following change?
   
   -    np_batch = [np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch]
   +    np_batch = [
   +        np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch
   +    ]



##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -75,9 +75,14 @@ def run_inference(
       model: PyODBaseDetector,
       inference_args: Optional[dict[str, Any]] = None
   ) -> Iterable[PredictionResult]:
-    np_batch = []
-    for row in batch:
-      np_batch.append(np.fromiter(row, dtype=np.float64))
+    def _flatten_row(row_values):
+      for value in row_values:
+        if isinstance(value, (list, tuple, np.ndarray)):
+          yield from value
+        else:
+          yield value
+
+    np_batch = [np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch]

Review Comment:
   This line is too long and causing tests on linter and formatter failed. 
   
   Could you make the following change?
   
   ```python
   -    np_batch = [np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch]
   +    np_batch = [
   +        np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch
   +    ]
   ```



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "gemini-code-assist[bot] (via GitHub)" <gi...@apache.org>.
gemini-code-assist[bot] commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2280345297


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -77,7 +77,13 @@ def run_inference(
   ) -> Iterable[PredictionResult]:
     np_batch = []
     for row in batch:
-      np_batch.append(np.fromiter(row, dtype=np.float64))
+      features = []
+      for value in row:
+        if isinstance(value, (list, tuple, np.ndarray)):
+          features.extend(value)
+        else:
+          features.append(value)
+      np_batch.append(np.array(features, dtype=np.float64))

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The current implementation is correct, but it can be made more efficient and concise.
   It currently builds an intermediate list `features` for each row, and uses `np.array()` which can be slower than `np.fromiter()` for creating 1D arrays from iterables.
   
   A more performant approach is to use a generator function to flatten features on-the-fly and pass it to `np.fromiter`. This avoids creating the intermediate list for each row.
   
   ```python
       def _flatten_row(row_values):
         for value in row_values:
           if isinstance(value, (list, tuple, np.ndarray)):
             yield from value
           else:
             yield value
       np_batch = [np.fromiter(_flatten_row(row), dtype=np.float64) for row in batch]
   ```



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #35884: fix: support vector features in PyOD anomaly detection batching logic
URL: https://github.com/apache/beam/pull/35884


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3198886806

   I think you are using your own formatter which is causing the linter and formatter problem.
   
   Could you run `tox -c tox.ini run -e py3-yapf` under `<YOUR_PROJECT_DIR>/sdks/python`? That should fix the formatting issue.


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3192763293

   Assigning reviewers:
   
   R: @tvalentyn for label python.
   
   
   Note: If you would like to opt out of this review, comment `assign to next reviewer`.
   
   Available commands:
   - `stop reviewer notifications` - opt out of the automated review tooling
   - `remind me after tests pass` - tag the comment author after tests pass
   - `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
   
   The PR bot will only process comments in the main thread (not review comments).


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "vijay-x-Raj (via GitHub)" <gi...@apache.org>.
vijay-x-Raj commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3193686743

   should I implement the changes suggested by the bot.


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3210655672

   Hey @vijay-x-Raj, I see you are trying to fix the linter and format issue.
   
   Have you ever tried the comment I suggested at https://github.com/apache/beam/pull/35884#issuecomment-3198886806?
   
   We used a specific yapf config to ensure our code format is consistent across the project. Please run that command, otherwise the formatter test would fail:
   
   https://github.com/apache/beam/actions/runs/17105155269/job/48512023797?pr=35884
   
   After the formatter error is fixed, then we can check the logs in the linter test if there is any error:
   
   https://github.com/apache/beam/actions/runs/17105155251/job/48512023540?pr=35884
   
   Hope this helps!
   
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "tvalentyn (via GitHub)" <gi...@apache.org>.
tvalentyn commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3197266287

   R: @shunping 


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3217001520

   ## [Codecov](https://app.codecov.io/gh/apache/beam/pull/35884?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `10.00000%` with `27 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 56.71%. Comparing base ([`14bb49c`](https://app.codecov.io/gh/apache/beam/commit/14bb49c50aa747d560b556d601a39de640aa7c17?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`33c3295`](https://app.codecov.io/gh/apache/beam/commit/33c3295f469c9a0c019a0e82e1d7b1c2e5d65f9d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 30 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/beam/pull/35884?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...n/apache\_beam/ml/anomaly/detectors/pyod\_adapter.py](https://app.codecov.io/gh/apache/beam/pull/35884?src=pr&el=tree&filepath=sdks%2Fpython%2Fapache_beam%2Fml%2Fanomaly%2Fdetectors%2Fpyod_adapter.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvYW5vbWFseS9kZXRlY3RvcnMvcHlvZF9hZGFwdGVyLnB5) | 10.00% | [27 Missing :warning: ](https://app.codecov.io/gh/apache/beam/pull/35884?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #35884      +/-   ##
   ============================================
   + Coverage     56.69%   56.71%   +0.02%     
     Complexity     3380     3380              
   ============================================
     Files          1204     1220      +16     
     Lines        184114   184843     +729     
     Branches       3507     3507              
   ============================================
   + Hits         104379   104833     +454     
   - Misses        76406    76681     +275     
     Partials       3329     3329              
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/beam/pull/35884/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [python](https://app.codecov.io/gh/apache/beam/pull/35884/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.79% <10.00%> (-0.18%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/beam/pull/35884?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3456304837

   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3193690226

   > should I implement the changes suggested by the bot.
   
   If it makes sense, why not? I would like to state as well I am not familiar with this change. I just wanted to initiate some momentum on reviewing this PR
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "vijay-x-Raj (via GitHub)" <gi...@apache.org>.
vijay-x-Raj commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3210881896

   @shunping Sorry I tried that earlier but the format became weird that's why I applied changes manually afterwards .This time I thing it is working thanks for helping me out.
   


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "mohamedawnallah (via GitHub)" <gi...@apache.org>.
mohamedawnallah commented on PR #35884:
URL: https://github.com/apache/beam/pull/35884#issuecomment-3211189428

   @vijay-x-Raj – The formatter CI pipeline passed now 🎉
   
   The linter failing with this err (from publicly available CI logs)
   ```
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:142: error: Argument 1 to "OfflineDetector" has incompatible type "ModelHandler[tuple[Any, Row], tuple[Any, AnomalyPrediction], Union[Any, _ModelManager]]"; expected "KeyedModelHandler[Any, Row, Never, Any]"  [arg-type]
   apache_beam/ml/anomaly/detectors/pyod_adapter.py:145: error: Unused "type: ignore" comment  [unused-ignore]
   Found 2 errors in 1 file (checked 1334 source files)
   error: mypy exited with status 1
   ```
   
   It is okay. It takes a bit of time to go through the process of fixing linting issues especially on first contributions. You can fix this and perhaps you can test it utilizing `.pylintrc` and `mypy.ini` (don't remember the exact command to run against those files but those I think can be easily found online). This can give you a fast iteration of feedback locally instead of (fix an issue that cause the linting to fail -> push to CI -> async waiting for CI to finish -> oops there are still CI linting issues -> loop).
   https://github.com/apache/beam/blob/master/sdks/python/.pylintrc
   https://github.com/apache/beam/blob/master/sdks/python/mypy.ini


-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2292488982


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -29,6 +35,7 @@
 from apache_beam.ml.anomaly.specifiable import specifiable
 from apache_beam.ml.anomaly.thresholds import FixedThreshold
 from apache_beam.ml.inference.base import KeyedModelHandler
+from typing import cast, Any

Review Comment:
   Please remove "Any" and move this line after "from typing import Optional" (line 28).
   
   Imports needs to be placed in order and system imports come first, then third-party imports and apache beam imports.



-- 
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: github-unsubscribe@beam.apache.org

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

Re: [PR] fix: support vector features in PyOD anomaly detection batching logic [beam]

Posted by "shunping (via GitHub)" <gi...@apache.org>.
shunping commented on code in PR #35884:
URL: https://github.com/apache/beam/pull/35884#discussion_r2292488982


##########
sdks/python/apache_beam/ml/anomaly/detectors/pyod_adapter.py:
##########
@@ -29,6 +35,7 @@
 from apache_beam.ml.anomaly.specifiable import specifiable
 from apache_beam.ml.anomaly.thresholds import FixedThreshold
 from apache_beam.ml.inference.base import KeyedModelHandler
+from typing import cast, Any

Review Comment:
   Please put this line after "from typing import Optional".
   
   Imports needs to be placed in order and system imports come first, then third-party imports and apache beam imports.



-- 
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: github-unsubscribe@beam.apache.org

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