New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Optional event_col in AAF and CPH fitters #120
Conversation
@@ -267,7 +268,7 @@ def test_survival_table_from_events_raises_value_error_if_too_early_births(): | |||
min_obs = T.copy() | |||
min_obs[1] = min_obs[1] + 10 | |||
with pytest.raises(ValueError): | |||
df = survival_table_from_events(T, C, min_obs) | |||
survival_table_from_events(T, C, min_obs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
linting
@@ -422,7 +422,7 @@ def __init__(self, fit_intercept=True, alpha=0.95, penalizer=0.5): | |||
self.penalizer = penalizer | |||
assert penalizer >= 0, "penalizer must be >= 0." | |||
|
|||
def fit(self, dataframe, duration_col="T", event_col="E", | |||
def fit(self, dataframe, duration_col, event_col=None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like that duration_col is not assumed to be called 'T' anymore either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 yea not sure why I made the decision in the first place
Looks good otherwise |
Optional event_col in AAF and CPH fitters
@spacecowboy, this PR adds the option for the
event_col
parameter (default None). If left asNone
, then we assume all non-censored (similar to the kmf fitter).Also,
duration_col
is now a mandatory argument.