Skip to content

Conversation

@megatron-me-uk
Copy link
Contributor

Implement a test for SPARK-6551.

Implement a test for SPARK-6551.
@srowen
Copy link
Member

srowen commented Aug 5, 2015

(Can you add a little more to the title, like what the underlying JIRA is about?)

@megatron-me-uk megatron-me-uk changed the title [SPARK-6551][PYSPARK] [SPARK-6551][PYSPARK] Incorrect aggregate results if seqOp(...) mutates its first argument Aug 5, 2015
@megatron-me-uk
Copy link
Contributor Author

I believe that this will now resolve the issues reported in SPARK-6551 and test for regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment needs to be updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or removed.

@SparkQA
Copy link

SparkQA commented Aug 5, 2015

Test build #1363 has finished for PR 7965 at commit 140f050.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class BlockMatrix(DistributedMatrix):

@JoshRosen
Copy link
Contributor

/cc @davies, I think you had some comments regarding whether zeroValue could be copied over at #7378?

@davies
Copy link
Contributor

davies commented Aug 5, 2015

This bug is fixed by #7378 (merged into 1.4 and 1.5) , Could you send a patch against 1.3 branch (remove the unrelated changes)?

@megatron-me-uk
Copy link
Contributor Author

#7378 has been merged into master and fails half of the tests that I have added in this pull request. The returned result is correct however the zeroValue value supplied is changed. Is it desirable to change the zeroValuethat is supplied to rdd.aggregate?

@davies
Copy link
Contributor

davies commented Aug 5, 2015

@megatron-me-uk I think it's reasonable to change the zeroValue once you provide a mutable function, for example, the builtin function reduce also works in this way.

If we change to reply on deepcopy, it will not work in some cases (for example, object from C extension).

@megatron-me-uk
Copy link
Contributor Author

OK good point, I guess I will close this pull request then. I think the best way to resole this jira issue is to backport #7378 or just recommend an upgrade to 1.4.

@megatron-me-uk megatron-me-uk deleted the patch-4 branch August 6, 2015 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants