-
Notifications
You must be signed in to change notification settings - Fork 60
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
[GSProcessing] Add saving and re-applying for numerical transforms. #1085
Conversation
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Outdated
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Outdated
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Outdated
Show resolved
Hide resolved
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.
Please add .. versionadded:: 0.4.0
or .. versionchanged:: 0.4.0
for new and updated APIs.
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
Co-authored-by: xiang song(charlie.song) <[email protected]>
Co-authored-by: jalencato <[email protected]>
graphstorm-processing/graphstorm_processing/data_transformations/dist_feature_transformer.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Outdated
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Outdated
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
...hstorm_processing/data_transformations/dist_transformations/dist_numerical_transformation.py
Show resolved
Hide resolved
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.
LGTM
…ns/dist_transformations/dist_numerical_transformation.py Co-authored-by: xiang song(charlie.song) <[email protected]>
None of the APIs changed are exposed to users @classicsong do we want to track all internal changes? |
No. |
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.
LGTM
Issue #, if available:
Fixes #985
Description of changes:
_apply_standard_transform
,_apply_minmax_transform
), which we can call from both the original transformation and the re-applied one. The presence or absence of pre-computed statistics in the function call determines which code path we follow.apply_imputation
andapply_norm
functions to also return the representation along with the transformed DF. We encapsulate the return values in their own dataclass (ImputationResult
,NormalizationResult
), to make future modifications easier (by not requiring to change the function's return type).By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.