(Commit hash 948e415) CLONE fields in Diversity.R need to be hard-coded and simplified

Issue #77 closed
Roy Jiang created an issue
  1. Confusing variable renaming in line 862 Diversity.R of clone variable.
  2. "It’s probably best to go ahead and rename the private function arguments too" (e.g. helperAlpha, helperBeta, helperTesting) e.g. clone="CLONE" by default internally.
  3. Change slot names in objects to clone_id?

Comments (8)

  1. Jason Vander Heiden

    Also applies to the grouping column. As it is currently written, this could be hardcoded to “GROUP” and simplify the data structures.

    We’ll need to decide how grouping for beta diversity is intended to work though. Right now, it’s just calculated on mutually exclusive subgroups of grouping variable used for depth normalization, so a single hardcoded “GROUP” field would work for that. However, I’m not convinced beta diversity should be restricted to those conditions. If we want to support non-mutually exclusive groupings for beta diversity, then we’ll need to keep he grouping variable abstraction to support multiple grouping columns.

    (Unless we do something like “GROUP1” and “GROUP2”, but I don’t think that really gets us anything in terms of reducing complexity.)

  2. Jason Vander Heiden

    I think I misunderstood the bit about the helpers yesterday… If the object they take in uses “CLONE”, then I’d leave the default `clone=”CLONE”`.

  3. ssnn

    FYI I am changing to lower case fields that get exposed to users, to keep everything lower case matching the airr format. Based on this issue, I guess it would be good that both of you checked the updates in diversity and abundance.

  4. Jason Vander Heiden

    Seems to run fine, but the internal column names for data.frames stored in the classes are still upper case. For example:

    estimateAbundance(ExampleDb, group="sample_id", nboot=100)

    Slot "clone_by":
    [1] "CLONE"

    Slot "group_by":
    [1] "sample_id"

  5. ssnn

    I think I fixed that in some other commit, because this is what I get:

     Slot "clone_by":
    [1] "clone_id"
    
    Slot "group_by":
    [1] "sample_id"
    
    Slot "groups":
    [1] "-1h" "+7d"
    

  6. Log in to comment