TheNeuralBit commented on a change in pull request #15833:
URL: https://github.com/apache/beam/pull/15833#discussion_r745876678



##########
File path: sdks/python/apache_beam/dataframe/frames_test.py
##########
@@ -651,6 +651,18 @@ def test_series_getitem(self):
     s.index = s.index.map(float)
     self._run_test(lambda s: s[1.5:6], s)
 
+  def test_series_truncate(self):
+    s = pd.Series(['a', 'b', 'c', 'd', 'e', 'f'])
+    self._run_test(lambda s: s.truncate(before=1, after=3), s)
+
+  def test_dataframe_truncate(self):
+    df = pd.DataFrame({
+        'A': list('abcde'), 'B': list('fghij'), 'C': list('klmno')
+    },
+                      index=[1, 2, 3, 4, 5])
+    self._run_test(lambda df: df.truncate(before=1, after=3), df)
+    self._run_test(lambda df: df.truncate(before='A', after='B', axis=1), df)

Review comment:
       I think there's a subtle bug in the axis=1 case: if the columns aren't 
already in sorted order, your implementation will sort them. If you change this 
test like so, it should reveal that bug:
   
   ```suggestion
       df = pd.DataFrame({
           'C': list('abcde'), 'B': list('fghij'), 'A': list('klmno')
       },
                         index=[1, 2, 3, 4, 5])
       self._run_test(lambda df: df.truncate(before=1, after=3), df)
       self._run_test(lambda df: df.truncate(before='A', after='B', axis=1), df)
   ```
   
   I think the best solution is to create a different expression for axis=1 vs 
axis=0, and only do the `sort_index()` in the axis=0 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: [email protected]

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


Reply via email to