itholic commented on code in PR #37294:
URL: https://github.com/apache/spark/pull/37294#discussion_r930533991


##########
python/pyspark/pandas/frame.py:
##########
@@ -6318,10 +6318,7 @@ def pivot_table(
                 )
                 psdf = DataFrame(internal)
         else:
-            if isinstance(values, list):
-                index_values = values[-1]

Review Comment:
   This condition checks if the `index is None` and `isinstance(values, list)`.
   
   However, it's already filtered in the above path:
   ```python
           if isinstance(values, list) and index is None:
               raise NotImplementedError("values can't be a list without 
index.")
   ```
   
   So this condition never be the `True`, until we implement the missing 
support.



##########
python/pyspark/pandas/frame.py:
##########
@@ -11116,7 +11131,7 @@ def mapper_fn(x: Any) -> Any:
             num_indices = len(index_columns)
             if level:
                 if level < 0 or level >= num_indices:
-                    raise ValueError("level should be an integer between [0, 
num_indices)")
+                    raise ValueError("level should be an integer between [0, 
%s)" % num_indices)

Review Comment:
   This just for improving the error message.



##########
python/pyspark/pandas/frame.py:
##########
@@ -12747,7 +12762,7 @@ def resample(
         if on is None and not isinstance(self.index, DatetimeIndex):
             raise NotImplementedError("resample currently works only for 
DatetimeIndex")
         if on is not None and not isinstance(as_spark_type(on.dtype), 
TimestampType):
-            raise NotImplementedError("resample currently works only for 
TimestampType")
+            raise NotImplementedError("`on` currently works only for 
TimestampType")

Review Comment:
   ditto



##########
python/pyspark/pandas/series.py:
##########
@@ -7052,7 +7052,7 @@ def resample(
         if on is None and not isinstance(self.index, DatetimeIndex):
             raise NotImplementedError("resample currently works only for 
DatetimeIndex")
         if on is not None and not isinstance(as_spark_type(on.dtype), 
TimestampType):
-            raise NotImplementedError("resample currently works only for 
TimestampType")
+            raise NotImplementedError("`on` currently works only for 
TimestampType")

Review Comment:
   Improving the error message.



##########
python/pyspark/pandas/frame.py:
##########
@@ -12840,7 +12855,6 @@ def __getitem__(self, key: Any) -> Any:
             return self.loc[:, key]
         elif is_list_like(key):
             return self.loc[:, list(key)]
-        raise NotImplementedError(key)

Review Comment:
   All possible cases are filtered in the above conditions, so this path will 
never be used.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to