Github user srowen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20632#discussion_r168936124
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/tree/Node.scala ---
    @@ -287,6 +292,41 @@ private[tree] class LearningNode(
         }
       }
     
    +  /**
    +   * Method testing whether a node is a leaf.
    +   * @return true iff a node is a leaf.
    +   */
    +  private def isLeafNode(): Boolean = leftChild.isEmpty && 
rightChild.isEmpty
    +
    +  /** True iff the node should be a leaf. */
    +  private lazy val shouldBeLeaf: Boolean = leafPredictions.size == 1
    +
    +  /**
    +   * Returns the set of (leaf) predictions appearing in the subtree rooted 
at the considered node.
    +   * @return the set of (leaf) predictions appearing in the subtree rooted 
at the given node.
    +   */
    +  private def leafPredictions: Set[Double] = {
    +
    +    val predBuffer = new scala.collection.mutable.HashSet[Double]
    +
    +    // collect the (leaf) predictions in the left subtree, if any
    +    if (leftChild.isDefined) {
    --- End diff --
    
    No, I'm just thinking you use `Option.foreach(...)` twice. It's a matter of 
taste I guess. I tend to prefer avoiding the `isDefined` and `get` calls for 
simple one-liners.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to