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

[SPARK-11380][Docs] Replace example code in mllib-frequent-pattern-mining.md using include_example #9340

Closed
wants to merge 6 commits into from

Conversation

pravingadakh
Copy link
Contributor

No description provided.

@pravingadakh pravingadakh changed the title replaced example code using include_example [SPARK-11380][Docs] replaced example code using include_example Oct 28, 2015
@mengxr
Copy link
Contributor

mengxr commented Oct 29, 2015

@pravingadakh Could you update the PR title to make it more specific?
@yinxusen Could you review this PR?

@mengxr
Copy link
Contributor

mengxr commented Oct 29, 2015

ok to test

@pravingadakh pravingadakh changed the title [SPARK-11380][Docs] replaced example code using include_example [SPARK-11380][Docs] Replace example code in mllib-frequent-pattern-mining.md using include_example Oct 29, 2015
@yinxusen
Copy link
Contributor

@pravingadakh You need to label $example on$ and $example off$ in comments of example codes. Otherwise, the include_example cannot trim the code snippt out. Example here: https://github.com/apache/spark/pull/9266/files#diff-deb98281f55f2891e75bae91cfe99d43R20

After finishing the work, you can go to spark/docs, install jekyll according to the README.md file. Then you can use SKIP_API=1 jekyll serve --watch to generate the webpages then go to localhost:4000 to check the result. :)

import org.apache.spark.api.java.JavaSparkContext;
import org.apache.spark.mllib.fpm.AssociationRules;
import org.apache.spark.mllib.fpm.FPGrowth;
import java.util.Arrays;
Copy link
Contributor

Choose a reason for hiding this comment

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

order of imports

@pravingadakh
Copy link
Contributor Author

@yinxusen I will do the changes mentioned. I am confused about one thing though, FPGrowth examples are already available in scala and java language, and it's content does not match with whatever is there in doc. What should be done here? Should I create a new file with example used in doc?

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44571 has finished for PR 9340 at commit fb0050b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * public class JavaAssociationRulesExample\n * public class JavaPrefixSpanExample\n * class HasSolver(Params):\n

@yinxusen
Copy link
Contributor

@pravingadakh I perfer to use the existing example code. @mengxr What is your opinion?

@pravingadakh
Copy link
Contributor Author

@yinxusen When you say use the existing example code, you mean the one in the doc or the one which is readily available in examples directory?

@pravingadakh
Copy link
Contributor Author

Jenkins test this please

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44597 has finished for PR 9340 at commit ab42465.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * public class JavaAssociationRulesExample\n * public class JavaPrefixSpanExample\n * public class JavaSimpleFPGrowth\n

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44598 has finished for PR 9340 at commit ab31ba4.

  • This patch fails Python style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * public class JavaAssociationRulesExample\n * public class JavaPrefixSpanExample\n * public class JavaSimpleFPGrowth\n

@pravingadakh
Copy link
Contributor Author

Jenkins test this please

@mengxr
Copy link
Contributor

mengxr commented Oct 29, 2015

@pravingadakh You can run dev/lint-scala and dev/lint-python to check code style locally.

@mengxr
Copy link
Contributor

mengxr commented Oct 29, 2015

@yinxusen Let's make this PR simple. Just move the example code from user guide to examples. We can have a follow-up PR to merge it with existing example app code.

@pravingadakh
Copy link
Contributor Author

@mengxr Ran the style checks locally, passed. Also I have moved FPGrowth example code from user guide to examples.

package org.apache.spark.examples.mllib

// $example on$
import org.apache.spark.rdd.RDD
Copy link
Contributor

Choose a reason for hiding this comment

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

swap this two lines. imports in one group should follow alphabetic order, see here: https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-Imports

import java.util.Arrays;
import java.util.List;

import org.apache.spark.mllib.fpm.PrefixSpan;
Copy link
Contributor

Choose a reason for hiding this comment

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

reorgnize the import. we can use multi $example on$ -- $example off$ blocks to select our imports.

@yinxusen
Copy link
Contributor

@mengxr Everything is OK except for some style problems. @pravingadakh Thanks for working on this.

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44621 has finished for PR 9340 at commit 2ca85d8.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * public class JavaAssociationRulesExample\n * public class JavaPrefixSpanExample\n * public class JavaSimpleFPGrowth\n

@SparkQA
Copy link

SparkQA commented Oct 29, 2015

Test build #44622 has finished for PR 9340 at commit 6f09636.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * public class JavaAssociationRulesExample\n * public class JavaPrefixSpanExample\n * public class JavaSimpleFPGrowth\n

@pravingadakh
Copy link
Contributor Author

@yinxusen Done with all import re-orderings.

@pravingadakh
Copy link
Contributor Author

@mengxr @yinxusen Can we merge this, if there aren't any further issues?

@mengxr
Copy link
Contributor

mengxr commented Nov 4, 2015

Merged into master. Thanks! @yinxusen Please use LGTM to sign off so there is a green box associated with this PR on https://spark-prs.appspot.com/#mllib.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants