Skip to content
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

BooleanType test shouldn't xfail #639

Merged
merged 10 commits into from
Sep 4, 2020

Conversation

razajafri
Copy link
Collaborator

@razajafri razajafri commented Sep 2, 2020

Signed-off-by: Raza Jafri rjafri@nvidia.com

Now that https://github.com/apache/spark/pull/29506/files has been merged to branch 3.0 in spark. This is no longer needed

fixes #350

Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@sameerz sameerz added this to the Aug 31 - Sep 11 milestone Sep 2, 2020
@sameerz sameerz added the test Only impacts tests label Sep 2, 2020
Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

@revans2 I have addressed your concern PTAL

@revans2
Copy link
Collaborator

revans2 commented Sep 3, 2020

build

@@ -61,7 +62,7 @@ def test_passing_gpuExpr_as_Expr():
@pytest.mark.parametrize('join_type', ['Left', 'Right', 'Inner', 'LeftSemi', 'LeftAnti'], ids=idfn)
@ignore_order
def test_cache_join(data_gen, join_type):
if data_gen.data_type == BooleanType():
if spark_version() == "3.0.0" and data_gen.data_type == BooleanType():
pytest.xfail("https://github.com/NVIDIA/spark-rapids/issues/350")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would point to the spark issue for all of these instead

https://issues.apache.org/jira/browse/SPARK-32672

@@ -41,3 +41,5 @@ def get_spark_i_know_what_i_am_doing():
"""
return _spark

def spark_version():
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, but there are a number of other places in the code that are doing a

with_spark_session(spark: spark.sparkContext.version ...

It would probably be good to update all of them to be consistent or do it the same way here too.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have reverted the change to be more like the rest to be consistent

Signed-off-by: Raza Jafri <rjafri@nvidia.com>
@razajafri
Copy link
Collaborator Author

build

revans2
revans2 previously approved these changes Sep 3, 2020
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is consistent now, but I like your new function better.

@razajafri
Copy link
Collaborator Author

OK. let me make the change. I thought making the PR bigger will be frowned upon.

@razajafri
Copy link
Collaborator Author

build

@razajafri
Copy link
Collaborator Author

build

@razajafri razajafri merged commit 9c4070c into NVIDIA:branch-0.2 Sep 4, 2020
@razajafri razajafri deleted the boolean-fix-cache branch September 4, 2020 02:34
@revans2
Copy link
Collaborator

revans2 commented Sep 8, 2020

build

nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* BooleanType test shouldn't xfail

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* Revert "BooleanType test shouldn't xfail"

This reverts commit 64714a5.

* xfail only for 3.0.0

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* addressed review comments

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* removed the version method

* Revert "removed the version method"

This reverts commit a9d9a04.

* Revert "addressed review comments"

This reverts commit 6472e92.

* added version checks

* missed changing it in cache_test.py

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* removing unnecessary lambda

Co-authored-by: Raza Jafri <rjafri@nvidia.com>
nartal1 pushed a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
* BooleanType test shouldn't xfail

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* Revert "BooleanType test shouldn't xfail"

This reverts commit 64714a5.

* xfail only for 3.0.0

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* addressed review comments

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* removed the version method

* Revert "removed the version method"

This reverts commit a9d9a04.

* Revert "addressed review comments"

This reverts commit 6472e92.

* added version checks

* missed changing it in cache_test.py

Signed-off-by: Raza Jafri <rjafri@nvidia.com>

* removing unnecessary lambda

Co-authored-by: Raza Jafri <rjafri@nvidia.com>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
…IDIA#639)

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Only impacts tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Integration Cache tests fails for Boolean values
3 participants