Feature/one hot encoder default int8 #175
Conversation
There was a problem hiding this comment.
Looks good. Comments on duplicate changes from #173 and changelog update needing fixing.
CHANGELOG.rst
Outdated
| ------------------ | ||
| Added | ||
| ^^^^^ | ||
| - Update OneHotEncodingTransformer to default to returning int8 columns https://github.com/lvgig/tubular/pull/175 |
There was a problem hiding this comment.
please update link to same format as rest of changelog
There was a problem hiding this comment.
updated here 72cd91f
tubular/imputers.py
Outdated
| @@ -469,6 +469,6 @@ def transform(self, X: pd.DataFrame) -> pd.DataFrame: | |||
| X = super().transform(X) | |||
|
|
|||
There was a problem hiding this comment.
Did you mean to update NullIndicator in this PR as well? Happy to have both in 1 PR if you just want to abandon #173 and update this PR to reflect that both are here
There was a problem hiding this comment.
Or assuming changes are same in both might be simplest to just merge #173 in first since it looks fine. Should be fine so long as we don't squash commit.
There was a problem hiding this comment.
oops sorry, not sure how this happened, must have been in a rush on Friday. Sounds good, can complete 173 then revisit this one to see if there are any issues
Edited OneHotEncodingTransformer to default as outputting int8 types. Output cols should be all 0/1, so do not see a downside to this. Will be significant memory savings compared to default float64.