-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-32162][PYTHON][TESTS] Improve error message of Pandas grouped map test with window #28987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-32162][PYTHON][TESTS] Improve error message of Pandas grouped map test with window #28987
Conversation
|
This is currently being looked at in apache/arrow#7604 |
|
ping @HyukjinKwon please take a look, thanks! |
HyukjinKwon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Test build #124929 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #124992 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125000 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125011 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125020 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125028 has finished for PR 28987 at commit
|
|
I think .. the machines became very slow .. for some reasons. |
|
retest this please |
|
Test build #125031 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125050 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125061 has finished for PR 28987 at commit
|
|
retest this please |
|
Test build #125068 has finished for PR 28987 at commit
|
|
Woah, finally |
|
Merged to master and branch-3.0. |
…map test with window
### What changes were proposed in this pull request?
Improve the error message in test GroupedMapInPandasTests.test_grouped_over_window_with_key to show the incorrect values.
### Why are the changes needed?
This test failure has come up often in Arrow testing because it tests a struct with timestamp values through a Pandas UDF. The current error message is not helpful as it doesn't show the incorrect values, only that it failed. This change will instead raise an assertion error with the incorrect values on a failure.
Before:
```
======================================================================
FAIL: test_grouped_over_window_with_key (pyspark.sql.tests.test_pandas_grouped_map.GroupedMapInPandasTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/spark/python/pyspark/sql/tests/test_pandas_grouped_map.py", line 588, in test_grouped_over_window_with_key
self.assertTrue(all([r[0] for r in result]))
AssertionError: False is not true
```
After:
```
======================================================================
ERROR: test_grouped_over_window_with_key (pyspark.sql.tests.test_pandas_grouped_map.GroupedMapInPandasTests)
----------------------------------------------------------------------
...
AssertionError: {'start': datetime.datetime(2018, 3, 20, 0, 0), 'end': datetime.datetime(2018, 3, 25, 0, 0)}, != {'start': datetime.datetime(2020, 3, 20, 0, 0), 'end': datetime.datetime(2020, 3, 25, 0, 0)}
```
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Improved existing test
Closes #28987 from BryanCutler/pandas-grouped-map-test-output-SPARK-32162.
Authored-by: Bryan Cutler <[email protected]>
Signed-off-by: HyukjinKwon <[email protected]>
|
Wow, Jenkins must have been slow back to work after the long weekend.. Thanks for the help @HyukjinKwon ! |
What changes were proposed in this pull request?
Improve the error message in test GroupedMapInPandasTests.test_grouped_over_window_with_key to show the incorrect values.
Why are the changes needed?
This test failure has come up often in Arrow testing because it tests a struct with timestamp values through a Pandas UDF. The current error message is not helpful as it doesn't show the incorrect values, only that it failed. This change will instead raise an assertion error with the incorrect values on a failure.
Before:
After:
Does this PR introduce any user-facing change?
No
How was this patch tested?
Improved existing test