Skip to content

Conversation

@icexelloss
Copy link
Contributor

What changes were proposed in this pull request?

Add docstring to clarify default window frame boundaries with and without orderBy clause

How was this patch tested?

Manually generate doc and check.

@icexelloss
Copy link
Contributor Author

cc @jiangxb1987 @hvanhovell

@icexelloss
Copy link
Contributor Author

Added doc per discussion on mailing list. Sorry that I have not generated scaladoc before, what's the best way to generate scaladoc locally?

@icexelloss icexelloss closed this Apr 4, 2018
@icexelloss icexelloss reopened this Apr 4, 2018
@SparkQA
Copy link

SparkQA commented Apr 4, 2018

Test build #88896 has finished for PR 20978 at commit f96eaea.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Apr 4, 2018

Test build #88895 has finished for PR 20978 at commit 0b6f5fc.

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

@felixcheung
Copy link
Member

PRODUCTION=1 RELEASE_VERSION="$SPARK_VERSION" jekyll build
and https://github.com/apache/spark/tree/master/docs#api-docs-scaladoc-javadoc-sphinx-roxygen2-mkdocs

* Window.partitionBy("country").orderBy("date").rowsBetween(-3, 3)
* }}}
*
* @note When ordering is not defined, the default frame boundaries are (rowFrame,
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to improve the wording (feel free not to use this):

When ordering is not defined, an unbounded window frame (rowFrame, unboundedPreceding, unboundedFollowing) is used by default. When ordering is defined, a growing window frame (rangeFrame, unboundedPreceding, currentRow) is used by default.

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 your wording is better. Updated.

@icexelloss icexelloss changed the title [SPARK-23861][SQL][Doc] Clarify default window frame boundaries with and without orderBy clause [SPARK-23861][SQL][Doc] Clarify default window frame with and without orderBy clause Apr 5, 2018
@icexelloss
Copy link
Contributor Author

screen shot 2018-04-05 at 2 04 59 pm

@icexelloss
Copy link
Contributor Author

@felixcheung Thanks much for the tips! I was able to generate scaladoc using sbt unidoc but for some reason my local changes are not reflected in the generated doc (I tried deleted the generated do and regenerate, but still no luck). I wonder I am missing sth?

@SparkQA
Copy link

SparkQA commented Apr 5, 2018

Test build #88956 has finished for PR 20978 at commit 8fb7527.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@icexelloss
Copy link
Contributor Author

jenkins retest please

@HyukjinKwon
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Apr 6, 2018

Test build #88981 has finished for PR 20978 at commit 8fb7527.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@hvanhovell
Copy link
Contributor

retest this please

@hvanhovell
Copy link
Contributor

it makes very little sense that this keeps failing

@icexelloss
Copy link
Contributor Author

@hvanhovell @HyukjinKwon thanks for helping with the build! I think this PR is ready except for that I still couldn't figure out why sbt unidoc isn't producing the updated the doc. I could be reading wrong but it seems like it uses the github as source to build docs:
https://github.com/apache/spark/blob/master/project/SparkBuild.scala#L727

Do you have suggestion how do I build the doc from local source?

@SparkQA
Copy link

SparkQA commented Apr 6, 2018

Test build #88983 has finished for PR 20978 at commit 8fb7527.

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

@hvanhovell
Copy link
Contributor

Merging to master. Thanks!

@HyukjinKwon
Copy link
Member

LGTM :)

@HyukjinKwon
Copy link
Member

and .. I guess @hvanhovell is hitting a network issue? I just merged it into master.

@asfgit asfgit closed this in d766ea2 Apr 6, 2018
@hvanhovell
Copy link
Contributor

Yeah, I got distracted. Thanks for merging.

@icexelloss
Copy link
Contributor Author

Thanks all!

@felixcheung
Copy link
Member

felixcheung commented Apr 6, 2018 via email

robert3005 pushed a commit to palantir/spark that referenced this pull request Apr 7, 2018
… orderBy clause

## What changes were proposed in this pull request?

Add docstring to clarify default window frame boundaries with and without orderBy clause

## How was this patch tested?

Manually generate doc and check.

Author: Li Jin <[email protected]>

Closes apache#20978 from icexelloss/SPARK-23861-window-doc.
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