Improve and clean-up Scaladocs; resolves #184 #193

ruebot · Apr 6, 2018

GitHub issue(s):

#184

What does this Pull Request do?

Improve and clean-up Scaladocs so that we will have much improved Scaladocs for the next release.

Every single scala file was touched.

How should this be tested?

TravisCI should take care of the build test
mvn site -DskipTests and visit target/site/scaladocs/index.html with a browser
We should do a smoke test with some auk standard output since we pulled in #189 and #186.

Interested parties

@lintool I'd appreciate your eyes on the doc comments in the code.
@ianmilligan1 can you do the smoke test, and any other review that you'd like to do?

Before this is merged let me know since we're squashing and merging this, the commit message is going to be pretty crazy with all the commits we've had go into this.

@greebie++ for his work on this.

greebie · Apr 6, 2018

Just a small typo here. ext => text.

codecov · Apr 6, 2018

Codecov Report

Merging #193 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #193      +/-   ##
==========================================
- Coverage   67.34%   67.29%   -0.06%     
==========================================
  Files          32       32              
  Lines         637      636       -1     
  Branches      124      124              
==========================================
- Hits          429      428       -1     
  Misses        167      167              
  Partials       41       41

Impacted Files	Coverage Δ
src/main/scala/io/archivesunleashed/package.scala	`71.66% <ø> (ø)`	⬆️
...io/archivesunleashed/matchbox/NER3Classifier.scala	`0% <ø> (ø)`	⬆️
...in/scala/io/archivesunleashed/util/JsonUtils.scala	`100% <ø> (ø)`	⬆️
...ala/io/archivesunleashed/matchbox/ComputeMD5.scala	`100% <ø> (ø)`	⬆️
...la/io/archivesunleashed/matchbox/ExtractDate.scala	`60% <ø> (ø)`	⬆️
.../io/archivesunleashed/matchbox/ExtractDomain.scala	`100% <ø> (ø)`	⬆️
...io/archivesunleashed/matchbox/DetectLanguage.scala	`100% <ø> (ø)`	⬆️
.../scala/io/archivesunleashed/app/WriteGraphML.scala	`100% <ø> (ø)`	⬆️
...scala/io/archivesunleashed/ArchiveRecordImpl.scala	`81.57% <ø> (ø)`	⬆️
...la/io/archivesunleashed/matchbox/ExtractUrls.scala	`100% <ø> (ø)`	⬆️
... and 18 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3163ace...28fca42. Read the comment docs.

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/ArchiveRecordImpl.scala

@@ -29,6 +29,11 @@ import org.archive.io.arc.ARCRecord
 import org.archive.io.warc.WARCRecord
 import org.archive.util.ArchiveUtils

+/** ArchiveRecord.
+ *
+ *  @constructor an archive record.


I believe the style guide calls for a noun phrase without trailing period?
http://www.oracle.com/technetwork/java/javase/tech/index-137868.html

That's Javadocs, not Scaladocs.

...that said, it would be nice to have something other than "ArchiveRecord." here. Do you have a suggestion?

@lintool are you good with this as is, or should we change it? I think this is the only outstanding left before we can merge.

I had

The `ArchiveRecord` [class](https://docs.scala-lang.org/tour/classes.html) is used by `RecordLoader` to manage the ingestion of WARC and ARC files. It represents one record from a collection found in the file. It uses the Internet Archive's ARCRecord & WARCRecord APIs to extract data from web crawls such as the crawl date, content strings, urls and so on. `ArchiveRecord` is a generic record that covers data from both ARCRecord and WARCRecord format. AUT will convert either format into the generic `ArchiveRecord` class.``` in the original reference document. So maybe ```ArchiveRecord is a class for ingesting WARC and ARC files.``` or ```Used by RecordLoader to extract data from WARC and ARC files.```

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/app/ExtractEntities.scala

@@ -21,27 +21,35 @@ import io.archivesunleashed.matchbox.{NER3Classifier, RemoveHTML}
 import org.apache.spark.SparkContext
 import org.apache.spark.rdd.RDD

-/**
-  * Extracts entities
+/** Conducts Named Entity Recognition (NER) on a WARC or ARC file.


Conducts -> Performs?

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/app/ExtractEntities.scala

    */
  def extractFromRecords(iNerClassifierFile: String, inputRecordFile: String, outputFile: String, sc: SparkContext): RDD[(String, String, String)] = {
    val rdd = RecordLoader.loadArchives(inputRecordFile, sc)
      .map(r => (r.getCrawlDate, r.getUrl, RemoveHTML(r.getContentString)))
    extractAndOutput(iNerClassifierFile, rdd, outputFile)
  }

-  /**
+  /** Extracts NER entities from tuple-formatted derivatives scraped from a website.


NER stands for Named Entity Recognition, so either say "Extracts named entities" or "Performs NER"...

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/app/ExtractGraph.scala

+      *
+      * @param verticesPath Filepath for vertices output
+      * @param edgesPath Filepath for edges output
+      * @return Unit().


no trailing period.

Trailing period goes on @return, but not on @param.

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/app/ExtractPopularImages.scala

 object ExtractPopularImages {
+
+  /** Extracts the n most popular images from an RDD within a given size range.


lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/matchbox/ComputeImageSize.scala

+
+  /** Computes image size from a byte array using ImageIO.
+   *
+   * Used by `ExtractPopularImages` to calculate the size of


Is backtick notation properly interpreted by scaladocs?

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/matchbox/DetectMimeTypeTika.scala

+  /** Detect mime type(s) from an input string.
+   *
+   * @param content a string of content for which to detect the MimeType
+   * @return mimetype(s) (e.g. "text/html" or "application/xml") or "N/A".


does this UDF really return multiple MIME types?

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/matchbox/NER3Classifier.scala

  def apply(file: String) = {
    serializedClassifier = file
  }

+  /** Conducts NER classificiation based on NER Classifier.


Same comment as above...

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/matchbox/RemoveHTML.scala

 object RemoveHTML {
+
+  /** Remove HTML markup.


Use 3rd person (descriptive) not 2nd person (prescriptive).

http://www.oracle.com/technetwork/java/javase/tech/index-137868.html

(Here and elsewhere)

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/package.scala

    */
  implicit class WARecordRDD(rdd: RDD[ArchiveRecord]) extends java.io.Serializable {
-
+    /** Removes all non-html-based data (images, executables etc.) from html text*/


missing period at end.

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/package.scala

    def keepMimeTypes(mimeTypes: Set[String]) = {
      rdd.filter(r => mimeTypes.contains(r.getMimeType))
    }
-
+    /** Removes all data that does not have selected data


missing period.

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/package.scala

    def keepDate(dates: List[String], component: DateComponent = DateComponent.YYYYMMDD) = {
      rdd.filter(r => dates.contains(ExtractDate(r.getCrawlDate, component)))
    }
-
+    /** Removes all data but selected exact urls


URL, Url, urls - inconsistent usage... it was "Url" above...

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

lintool · Apr 9, 2018

src/main/scala/io/archivesunleashed/util/TweetUtils.scala

  implicit class JsonTweet(tweet: JValue) {
    implicit lazy val formats = org.json4s.DefaultFormats
-
+    /* Get Twitter status id. */


do you mean to have /** ... */ here?

lintool · Apr 9, 2018

lintool reviewed Apr 9, 2018

View changes

Minor comments, okay to merge after cleanup...

ruebot · Apr 9, 2018

ruebot dismissed lintool’s stale review via `abd4701` Apr 9, 2018

ianmilligan1 · Apr 9, 2018

ianmilligan1 reviewed Apr 9, 2018

View changes

Looks great to me!

lintool · Apr 10, 2018

lgtm

lintool · Apr 10, 2018

lintool reviewed Apr 10, 2018

View changes

ruebot · Apr 10, 2018

@lintool did you want me to add Used by RecordLoader to extract data from WARC and ARC files. in place of ArchiveRecord. above, or it looks good as it is right now?

lintool · Apr 10, 2018

either is fine.

ruebot · Apr 10, 2018

@lintool cool. I'll add it, and then worth with @ianmilligan1 on getting it merged. Thanks for reviewing!

ruebot · Apr 10, 2018

ruebot dismissed stale reviews from lintool and ianmilligan1 via `28fca42` Apr 10, 2018

greebie · Apr 10, 2018

greebie approved these changes Apr 10, 2018

View changes

review

Verified

This commit was signed with a verified signature.

ruebot Nick Ruest

GPG key ID: 417FAF1A0E1080CD Learn about signing commits

Loading status checks…

42fc757

forgot to add this!

Verified

This commit was signed with a verified signature.

ruebot Nick Ruest

GPG key ID: 417FAF1A0E1080CD Learn about signing commits

Loading status checks…

ce5350e

ruebot requested review from lintool and ianmilligan1 Apr 9, 2018

ruebot dismissed lintool’s stale review via abd4701 Apr 9, 2018

Review

Verified

This commit was signed with a verified signature.

ruebot Nick Ruest

GPG key ID: 417FAF1A0E1080CD Learn about signing commits

Loading status checks…

abd4701

review

Verified

This commit was signed with a verified signature.

ruebot Nick Ruest

GPG key ID: 417FAF1A0E1080CD Learn about signing commits

Loading status checks…

28fca42

ruebot dismissed stale reviews from lintool and ianmilligan1 via 28fca42 Apr 10, 2018

ianmilligan1 deleted the issue-184 branch Apr 11, 2018

archivesunleashed/aut

Join GitHub today

Improve and clean-up Scaladocs; resolves #184 #193

Conversation

ruebot commented Apr 6, 2018 • edited

What does this Pull Request do?

How should this be tested?

Interested parties

ruebot and others added some commits Mar 20, 2018

This comment has been minimized.

greebie commented on src/main/scala/io/archivesunleashed/matchbox/ExtractBoilerpipeText.scala in 40400ea Apr 6, 2018

This comment has been minimized.

codecov bot commented Apr 6, 2018 • edited

Codecov Report

ruebot requested review from lintool and ianmilligan1 Apr 9, 2018

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

greebie Apr 10, 2018 • edited

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

This comment has been minimized.

lintool reviewed Apr 9, 2018 View changes

lintool left a comment

ruebot dismissed lintool’s stale review via abd4701 Apr 9, 2018

ianmilligan1 reviewed Apr 9, 2018 View changes

ianmilligan1 left a comment

This comment has been minimized.

lintool commented Apr 10, 2018

lintool reviewed Apr 10, 2018 View changes

This comment has been minimized.

ruebot commented Apr 10, 2018

This comment has been minimized.

lintool commented Apr 10, 2018

This comment has been minimized.

ruebot commented Apr 10, 2018

ruebot dismissed stale reviews from lintool and ianmilligan1 via 28fca42 Apr 10, 2018

greebie approved these changes Apr 10, 2018 View changes

Hide details View details ianmilligan1 merged commit 47f7a97 into master Apr 11, 2018 2 of 4 checks passed

2 of 4 checks passed

ianmilligan1 deleted the issue-184 branch Apr 11, 2018

ruebot commented Apr 6, 2018 •

edited

greebie commented on `src/main/scala/io/archivesunleashed/matchbox/ExtractBoilerpipeText.scala` in `40400ea` Apr 6, 2018

codecov bot commented Apr 6, 2018 •

edited

lintool reviewed Apr 9, 2018

View changes

greebie Apr 10, 2018 •

edited

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

lintool reviewed Apr 9, 2018

View changes

ruebot dismissed lintool’s stale review via `abd4701` Apr 9, 2018

ianmilligan1 reviewed Apr 9, 2018

View changes

lintool reviewed Apr 10, 2018

View changes

ruebot dismissed stale reviews from lintool and ianmilligan1 via `28fca42` Apr 10, 2018

greebie approved these changes Apr 10, 2018

View changes

ianmilligan1 merged commit `47f7a97` into master Apr 11, 2018
2 of 4 checks passed