(Commit hash 948e415) CLONE fields in Diversity.R need to be hard-coded and simplified
- Confusing variable renaming in line 862 Diversity.R of clone variable.
- "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.
- Change slot names in objects to clone_id?
Comments (8)
-
-
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”`.
-
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.
-
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"
-
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"
-
ah, yeah, I was on the wrong branch.
-
- changed status to open
I am labeling this as open now, but please close if/when done.
-
- changed status to closed
- Log in to comment
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.)