Series.firstValue .lastValue

[dwhjames]

Series has the following methods:

def firstValue: Option[(K, V)] = {
  var i = 0
  while (i < index.size) {
    val row = index.indexAt(i)
    if (column.isValueAt(row))
      return Some(index.keyAt(i) -> column.valueAt(row))
    i += 1
  }
  None
}

def lastValue: Option[(K, V)] = {
  var i = index.size - 1
  while (i >= 0) {
    val row = index.indexAt(i)
    if (column.isValueAt(row))
      return Some(index.keyAt(i) -> column.valueAt(row))
    i -= 1
  }
  None
}

These are currently only used in one place:

def getFirstAndLastTopLevelMetricData(settings: ScaleSettings): EitherT[Future, MetricDataError, (List[Metric], Frame[Company, MetricColumn])] = {
  import MetricColumn.{ date, comparable }

  getTopLevelMetricData(settings) map { case (metadata, frame0) =>
    val metricKey = MetricColumn.Data(settings.metricsString(0))
    val frame = frame0.mapRowGroups { case (comp, group0) =>
      val group = group0.columns(date).groupAs[LocalDate]
      val values = group.column[Number](metricKey)
      val stripped = for {
        (date0, _) <- values.firstValue
        (date1, _) <- values.lastValue
      } yield {
        group.retainRows(date0, date1).columns(comparable).groupAs[Company]
      }
      stripped getOrElse Frame.empty[Company, MetricColumn]
    }

    (metadata, frame)
  }
}

My issue here is that these behave quite differently to the First and Last reducers.

1. they return keys as well as values
2. they ignore NM

We could add a new method to Series:

def findAsc[B](f: (K, Column[V], Int) => Option[B]): Option[B] = {
  var i = 0
  while (i < index.size) {
    val row = index.indexAt(i)
    val res = f(index.keyAt(i), column, row)
    if (res.isDefined)
      return res
    i += 1
  }
  None
}

Then firstValue becomes equivalent to:

def findFirstValue: Option[(K, V)] =
  findAsc((key, col, row) =>
    if (column.isValueAt(row))
      Some(key -> column.valueAt(row))
    else
      None
  )

Similarly, we would have findDesc and findLastKeyVal.

I guess there is a penalty for abstracting these traversals to findAsc and findDesc.

Thoughts?

[dwhjames]

Addressed in #16