aqp icon indicating copy to clipboard operation
aqp copied to clipboard

suggested changes to allocate

Open dylanbeaudette opened this issue 4 years ago • 4 comments

I can't figure out how to document all of the arguments to allocate such that arguments specific to sub-functions are clear in the manual page.

Ideas:

  • application of droplevels should probably happen after sub-functions are done, fewer arguments to pass around
  • sub-functions should be exported with their own documentation, manual page for allocate is generic with links
  • consider splitting fao_lev into two vectors (salinity and sodicity), matching by absolute position within fao_lev is tricky and breaks if classes change
  • consider optional output with saline / sodic classes in their own columns (result is a matrix or data.frame) so that cross-classification is possible e.g. "slightly saline / moderately sodic".
  • allocate(EC = NA, pH = 5, ESP = 1, to = 'FAO Salt Severity', droplevels = FALSE) results in an error

dylanbeaudette avatar Jan 14 '21 21:01 dylanbeaudette

Function details moved to Details section for now.

dylanbeaudette avatar Jan 19 '21 20:01 dylanbeaudette

I intend to open a PR linked to this issue with my suggestions and comments in it. I feel this will be easier to provide specific feedback on instances/lines of code. I am not going to do that before 1.27 goes to CRAN though -- and probably not before upcoming stats class -- so as to not detract from anything we need to do before that.

brownag avatar Jan 20 '21 16:01 brownag

Good work on fixing it up for this release though #179

brownag avatar Jan 20 '21 16:01 brownag

Hi @smroecker any thoughts on the above commentary / ideas for allocate? This won't prevent aqp 2.0 from being released, but now is a convenient time to make large changes.

dylanbeaudette avatar Apr 03 '23 18:04 dylanbeaudette