Skip to content

Conversation

@davies
Copy link
Contributor

@davies davies commented Sep 8, 2015

cc @mengxr

@SparkQA
Copy link

SparkQA commented Sep 8, 2015

Test build #42152 has finished for PR 8657 at commit 6548f25.

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

@rxin
Copy link
Contributor

rxin commented Sep 8, 2015

Would this also import the since function if somebody does "from pyspark import *"?

@davies
Copy link
Contributor Author

davies commented Sep 8, 2015

@rxin No, it's not in __all__.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be OK to add a clause to handle situations where f.doc is None? There are a handful of methods without docstrings. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's better to have some docs for public APIs.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, that makes more sense. Ignore the comment above.

@mengxr
Copy link
Contributor

mengxr commented Sep 9, 2015

LGTM. Merged into master. Thanks!

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