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


##########
sdks/python/apache_beam/dataframe/pandas_doctests_test.py:
##########
@@ -704,14 +701,19 @@ def test_groupby_tests(self):
     result = doctests.testmod(
         pd.core.groupby.groupby,
         use_beam=False,
+        verbose=True,
         wont_implement_ok={
+            'pandas.core.groupby.groupby.GroupBy.first': ['*'],

Review Comment:
   All of these (and other similar test cases were already not implemented, 
pandas just added more tests to exercise them).



##########
sdks/python/apache_beam/dataframe/frames.py:
##########
@@ -372,24 +372,24 @@ def last(self, offset):
   @frame_base.args_to_kwargs(pd.DataFrame)
   @frame_base.populate_defaults(pd.DataFrame)
   def groupby(self, by, level, axis, as_index, group_keys, **kwargs):
-    """``as_index`` and ``group_keys`` must both be ``True``.

Review Comment:
   group_keys support is required because the default behavior changed in 1.5 - 
https://pandas.pydata.org/pandas-docs/dev/whatsnew/v1.5.0.html#using-group-keys-with-transformers-in-dataframegroupby-apply-and-seriesgroupby-apply
 - this was caught by tests automatic pandas tests when trying to upgrade 
(which are now satisfied)



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