shunping commented on code in PR #41154:
URL: https://github.com/apache/beam/pull/41154#discussion_r4528297164


##########
sdks/python/apache_beam/io/textio_test.py:
##########
@@ -1889,6 +1890,48 @@ def check_types(element):
 
         _ = pcoll | beam.Map(check_types)
 
+  def test_float_precision_not_truncated(self):

Review Comment:
   I would suggest that we extend this test to cover a precision truncation 
scenario. It would also be great to add a short inline comment on the 
truncation case to explain why it is truncated.



##########
sdks/python/apache_beam/io/textio.py:
##########
@@ -1095,7 +1095,10 @@ def WriteToJson(
         as opposed to the entire file being a valid JSON object or list.
         Defaults to True if orient is 'records' (unlike Pandas).
       **kwargs: Extra arguments passed to `pandas.Dataframe.to_json`
-        (see below).
+        (see below). Unlike pandas, ``double_precision`` defaults to 15

Review Comment:
   I think this is a bit redundant with the inline comment below. Since the 
existing docstring already directs users to "see below", we could probably drop 
this addition here and just rely on the inline explanation.



##########
sdks/python/apache_beam/io/textio_test.py:
##########
@@ -1889,6 +1890,48 @@ def check_types(element):
 
         _ = pcoll | beam.Map(check_types)
 
+  def test_float_precision_not_truncated(self):
+    records = [
+        beam.Row(id=1, x=0.1234567890123),
+        beam.Row(id=2, x=1e-11),
+    ]
+    with tempfile.TemporaryDirectory() as dest:
+      with TestPipeline() as p:
+        # pylint: disable=expression-not-assigned
+        p | beam.Create(records) | beam.io.WriteToJson(
+            os.path.join(dest, 'out'))
+      written = []
+      for path in glob.glob(os.path.join(dest, 'out*')):
+        with open(path) as f:
+          written.extend(json.loads(line) for line in f if line.strip())
+    self.assertCountEqual(
+        written, [{
+            'id': 1, 'x': 0.1234567890123
+        }, {
+            'id': 2, 'x': 1e-11
+        }])
+
+  def test_double_precision_override(self):

Review Comment:
   This test looks share most of the code with the previous one. Have you 
considered consolidate them into one?



-- 
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