I30 dichotomy

[jwasserman2]

Major changes (in order of the files changed below)

c() for WeightedDesign objects

• Returns a numeric if all objects have the same Design slot
• Warns if warn_dichotomy_not_equal is TRUE (default is FALSE) and objects have the same Design slot but differing dichotomy slots
• Errors if objects have differing Design or target slots or if not all objects are WeightedDesign
  • Should it not error if a WeightedDesign is concatenated with a numeric? A: instead of an error, we should provide a message saying we're not checking whether Design or target are the same

Design objects

• No longer have dichotomy slots
• Delete dichotomy extractor function
• No more use of is_dichotomized
• Remove their reference in other functions such as identical_designs

treatment()/.bin_txt()

• .bin_txt() now accepts optional data and dichotomy arguments in addition to Design
• .bin_txt() logic:
  • Get the treatment stored in the Design using treatment(Design, binary = FALSE)
  • If data is not NULL, join treatment to data on unit of assignment information using .expand_txt() (existing code from assigned() that I made into a helper function). If dichotomy refers to a non-treatment variable, it must be provided in data
  • Should this join take/use a by argument?
  • If dichotomy is not NULL, dichotomize the treatment using .apply_dichotomy()
• treatment() now accepts dichotomy argument
• treatment() logic:
  • If binary == FALSE, return .get_col_from_new_data if newdata is not NULL or treatment column from Design if it is
  • If binary == TRUE and dichotomy is not NULL, return .bin_txt()
  • If binary == TRUE and dichotomy is NULL and has_binary_treatment(Design) is FALSE, error. Otherwise perform same logic as binary == FALSE

WeightedDesign objects

• Now have a dichotomy slot

assigned()

• Now has a dichotomy arugment
• Returns dichotomized treatment if dichotomy is provided, returns treatment stored in the Design if no data is provided, otherwise returns treatment joined to data
• Need it to error when data is NULL and .get_data_from_model doesn't find non-treatment variables in the dichotomy

.get_dichotomy()

• A new function for finding a dichotomy argument provided in lmitt, a WeightedDesign, or a call provided to the weights argument in lmitt. If .get_dichotomy() was provided a dichotomy itself, it compares that dichotomy against any found further up the stack

lmitt()

• Add dichotomy argument
• Check for dichotomy provided in the call or object provided to weights argument
• Allow for factor treatment if a dichotomy is provided
• Pass dichotomy argument to assigned() in the generated formula, but remove it from the column names of the model matrix

.weights_calc()

• **Is a data argument even necessary in the .bin_txt() call?

[josherrickson]

Per discussion with Josh:

• Print message along the lines of "concatenating WeightedDesign with numeric, checking of equality of Design is disabled"
• Remove is_dichotomized and is_binary_or_dichotomized` functions
• In treatment, remove binary= argument (equivalent to testing whether dichotomy= argument is NULL)
• .bin_txt - At least one of these must be true: a) the treatment is binary, or b) the dichotomy= argument must be non-NULL.
• rename .get_dichotomy to something like .handle_dichotomy or .process_dichotomy

[benthestatistician]

I'd like to request a small addition to the nice chunk of work that was recently added on this branch. Looking over the discussion and the tests, it's not immediately clear to me where we landed with regard to use of the CombinedWeightedDesign class. File test.CWD.R now has a number of instances of expect_false(inherits(foo, "CombinedWeightedDesign")), but no corresponding expect_true()'s (nor any expect_s4_class(foo, "CombinedWeightedDesign")'s). This suggests that we landed on not using it. But the title of the file suggests that it's a thing, as do the inline comments surrounding the class definition and various of my inline notes in test.CWD.R -- e.g. comments in this block.

Could someone just update the inline notes to clarify? Particularly in test.CWD.R, and maybe CWD.R. I imagine this could be either Josh W or Josh E. Please don't hesitate to trash inline notes of mine that no longer apply. If it's appropriate to remove the CWD definition, I'm OK with that too; whatever leaves us a clearer trail of crumbs when we find ourselves maintaining this in the future.

[jwasserman2]

I think it'd be clearest if we just delete the CombinedWeightedDesign class from CombinedWeightedDesign.R and any tests in test.CombinedWeightedDesign.R that reference it, so if people find that acceptable I can go ahead and do that

[josherrickson]

I agree, let's remove all traces of CombinedWeightedDesign if we're not going to use it.

[benthestatistician]

Makes sense to me. If indeed we'll remove all traces of CWD, I'd suggest indicating that clearly in the commit message, in the event that we should someday think of another use for some of that nice code.

Where JW says get rid of tests referencing CWD, I take him to mean get rid of the parts of those tests that reference CWD. The tests of combining WDs are probably worth keeping around.

If there'll no longer be a CWD class, perhaps the names of these files should be changed accordingly too: e.g. CombinedWeightedDesign.R -> combine_WeightedDesign.R, test.CombinedWeightedDesign.R -> test.combine_WeightedDesign.R.

[jwasserman2]

Commit eedc0c4 addresses the final string of comments