simplify the selection of years in the training_pipeline #142
Conversation
| outcome_years_to_keep = [ | ||
| year for year in outcome_years_to_keep if year in intervention.columns[2:] | ||
| ] | ||
|
|
There was a problem hiding this comment.
The line 106 to line 114 works the same as
outcome_years_to_keep = [year for year in outcome.columns[2:] if year in intervention.columns[2:]
It seems that the intent was to write
outcome_years_to_keep = [year for year in outcome.columns[2:] if year - forward_shift in intervention.columns[2:]
There was a problem hiding this comment.
Alright, I think the issue was dropping a variable that was needed elsewhere.
|
|
||
| for i in range(len(outcome_years_to_keep) - forward_shift): | ||
| outcome_shifted.iloc[:, i] = outcome_shifted.iloc[:, i + forward_shift] | ||
|
|
There was a problem hiding this comment.
The shifting could be completed in one step through renaming.
There was a problem hiding this comment.
Multiple tests fail as you removed a variable that is needed downstream. Also, changing the optimizer to abide by the lint is not recommended. It seems there is a bug in mypy where it doesn't seem to see things in torch.
So instead I'd recommend ignoring this line by mypy and reverting to the original optimizer.
If you feel like fixing this, please do, if your otherwise preoccupied, lmk, we can stall this PR.
| outcome_years_to_keep = [ | ||
| year | ||
| for year in outcome.columns[2:] | ||
| if year_min <= int(year) <= year_max + forward_shift | ||
| if str(int(year) - forward_shift) in intervention.columns[2:] | ||
| ] | ||
|
|
There was a problem hiding this comment.
I think this is what intended here but if you want to have the same behavior as before, change it to outcome_years_to_keep = [year for year in outcome.columns[2:] if year in intervention.columns[2:]]
rfl-urbaniak
dismissed
Niklewa’s stale review
Manually went over and agree. Minor issue no point in sending to re-review.
No description provided.