search

obrl-soil / mpspline2

A revamped mass-preserving spline function for soils data
6 stars 2 forks source link

tiny style suggestions for working with SoilProfileCollection objects #1

Closed dylanbeaudette closed 5 years ago

dylanbeaudette commented 5 years ago

Howdy. This is minor, but worth considering since the internal structure of SPC objects is subject to change. This is more important as aqp 2.0 looms (asymptotically?) closer.

The following code works fine but can / will break when internals change (sorry).

mpspline_conv.SoilProfileCollection <- function(obj = NULL) {
  dc <- obj@depthcols
  ic <- obj@idcol
  ac <- names(obj@horizons)[-which(names(obj@horizons) %in% c(dc, ic))]
  data.frame(c(obj@horizons[ic],
               obj@horizons[dc],
               obj@horizons[ac]), stringsAsFactors = FALSE)
}

Safely get everything you need like this (please). Comments / whitespace added for clarity.

# profile ID field name, set at SPC init time
ic <- idname(obj)

# horizon depth field names, set at SPC init time
dc <- horizonDepths(obj)

# horizon level attributes, includes IDs and depths
h <- horizons(obj)

# horizon level field names, minus IDs and depths
ac <- names(h)[-which(names(h) %in% c(ic, dc))]

# re-order to expected format: ID, top, bottom, {everything else}
h <- h[, c(ic, dc, ac)]

# done, stringsAsFactors logic mirrors source data
return(h)

I think that this accomplishes what is intended. A couple of things gained:

Let me know if I missed something.

obrl-soil commented 5 years ago

Aha yes, my bad S4 habits strike again :P all works well for me locally - do you want to do a PR and get your name on this thing?

dylanbeaudette commented 5 years ago

Took me a while to figure out the PR. I think that I did it correctly. devtools::test() seemed to run fine.

obrl-soil commented 5 years ago

Closing with fix from https://github.com/obrl-soil/mpspline2/pull/3