odsc-65115 : Transform columns names to make them compatible across models #1046
Conversation
| @@ -59,7 +60,8 @@ def run(self, data): | |||
| """ | |||
| clean_df = self._remove_trailing_whitespace(data) | |||
| # clean_df = self._normalize_column_names(clean_df) | |||
| if hasattr(self.dataset_info, 'horizon'): | |||
There was a problem hiding this comment.
Is this a proxy for "if it's forecasting"?
If so it may be better to use kind/type for future compatibility
There was a problem hiding this comment.
In the transformations we use OperatorSpec, incase of the forecasting it is ForecastOperatorSpec, type is available in the OperatorConfig which is not required here and adding type to spec will make it redundant, so I have doing that. Updated this to us operatorspec instance type check for better readability .
| self.target_category_columns = [ | ||
| self.raw_column_names.get(col, col) for col in self.target_category_columns | ||
| ] | ||
| df.columns = df.columns.str.replace(" ", "") |
There was a problem hiding this comment.
Should we be doing this replace 2x? Is there a way to do it once so we ensure it's done the same way
| @@ -226,6 +254,10 @@ def _check_historical_dataset(self, df): | |||
| expected_names = [self.target_column_name, self.dt_column_name] + ( | |||
| self.target_category_columns if self.target_category_columns else [] | |||
| ) | |||
| if self.raw_column_names: | |||
| expected_names.extend(list(self.raw_column_names.values())) | |||
There was a problem hiding this comment.
Are these guaranteed to be in historical data?
There was a problem hiding this comment.
yes, as we are making this transformation to the historical data columns, expected columns has to be extended using the same post transformed columns
odsc-65115
Add the column name transformations, also save a mapping for reverse transformation as per model requirement.