Skip to content

Conversation

@yanboliang
Copy link
Contributor

This PR including the following changes:

Todo:
If this PR has been accepted, I can do the peer renaming for Python and R in a follow-up PR.

cc @rxin @marmbrus @davies

@SparkQA
Copy link

SparkQA commented Dec 1, 2015

Test build #46936 has finished for PR 10056 at commit edf7e7b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think its fine to be uniform with SQL in the functions that we call with columns as argument, but this seems like here we are breaking user code for no reason. And if we are going to go lowercase then why are these isnull and not is_null?

For things that are just standard methods on objects I don't see a need to break with scala/java conventions. We can have aliases in python to be compatible with pandas if that is the motivation.

/cc @rxin

Copy link
Contributor

Choose a reason for hiding this comment

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

I left isnull unchanged in my earlier pull request because I felt it was too commonly used, and decided not to change them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@marmbrus @rxin Thanks for your comments. The conclusion is clear, I will close this PR and send follow-up one(#10097) to add alias at python side in order to be compatible with pandas.

@yanboliang yanboliang closed this Dec 2, 2015
@yanboliang yanboliang deleted the SPARK-12067 branch December 2, 2015 09:23
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.

4 participants