damccorm commented on code in PR #35387:
URL: https://github.com/apache/beam/pull/35387#discussion_r2161683527


##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -1445,30 +1445,50 @@ class LogElements(PTransform):
     level: (optional) The logging level for the output (e.g. `logging.DEBUG`,
         `logging.INFO`, `logging.WARNING`, `logging.ERROR`). If not specified,
         the log is printed to stdout.
+    with_pane_info (bool): (optional) Whether to include element's pane info.
+    use_rfc3339 (bool): (optional) Whether to display timestamps in rfc3339
+        format.
   """
   class _LoggingFn(DoFn):
     def __init__(
-        self, prefix='', with_timestamp=False, with_window=False, level=None):
+        self,
+        prefix='',
+        with_timestamp=False,
+        with_window=False,
+        with_pane_info=False,
+        use_rfc3339=True,
+        level=None):

Review Comment:
   Nit: Param order doesn't match order in pydoc



##########
sdks/python/apache_beam/transforms/util.py:
##########
@@ -1445,30 +1445,50 @@ class LogElements(PTransform):
     level: (optional) The logging level for the output (e.g. `logging.DEBUG`,
         `logging.INFO`, `logging.WARNING`, `logging.ERROR`). If not specified,
         the log is printed to stdout.
+    with_pane_info (bool): (optional) Whether to include element's pane info.
+    use_rfc3339 (bool): (optional) Whether to display timestamps in rfc3339

Review Comment:
   This parameter name is hard to understand, and it is surprising to have one 
param that defaults to True while the rest default to False. What if we named 
this `use_epoch_time`? I think people have a better shot of knowing what this 
means and it lets us keep the defaults to False.
   
   



-- 
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-unsubscr...@beam.apache.org

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

Reply via email to