Update Darts models and make log and unlog inside stepshifter#43
Update Darts models and make log and unlog inside stepshifter#43xiaolong0728 merged 11 commits intodevelopmentfrom
Conversation
xiaolong0728
commented
Nov 24, 2025
xiaolong0728
commented
- Darts now support classification models. Remove previous self-created models and import from darts
- Log and unlog data inside views-stepshifter. No transformed data in and out
There was a problem hiding this comment.
Pull request overview
This PR modernizes the Darts model usage and internalizes data transformations within the stepshifter. The main changes upgrade to Darts 0.38.0 which now includes native classification model support, eliminating the need for custom implementations. Additionally, log/expm1 transformations are now performed internally rather than requiring pre-transformed data inputs.
- Upgraded Darts dependency from ^0.30.0 to ^0.38.0 to leverage built-in classification models
- Moved log1p/expm1 transformations inside StepshifterModel's data processing and prediction methods
- Simplified darts_model.py by removing extensive custom model implementations and using thin wrappers instead
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| views_stepshifter/models/stepshifter.py | Added log1p transformation in _process_data and expm1 in _predict_by_step; updated deprecated pd_dataframe() to to_dataframe(); removed unused ModelManager import and commented out CUDA code |
| views_stepshifter/models/hurdle_model.py | Replaced custom classification models with native Darts imports (XGBClassifierModel, LightGBMClassifierModel) and removed RandomForestClassifier support |
| views_stepshifter/models/darts_model.py | Drastically reduced file from 1221 to 30 lines by replacing custom model implementations with simple wrapper classes that inherit from Darts models |
| views_stepshifter/models/init.py | Removed exports for deleted custom model classes (XGBClassifierModel, LightGBMClassifierModel, RandomForestClassifierModel) |
| views_stepshifter/manager/stepshifter_manager.py | Fixed typo: changed all references from 'config' to 'configs' to match the actual attribute name |
| tests/test_stepshifter_manager.py | Enhanced test setup with proper mocking, added validation patching, and corrected attribute references from 'config' to 'configs' |
| tests/test_hurdle_model.py | Updated sample config to use LGBM models instead of RandomForest models that were removed |
| pyproject.toml | Upgraded darts dependency from ^0.30.0 to ^0.38.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| self.config["model_reg"] = self.config["algorithm"] | ||
| model = StepshifterModel(self.config, partitioner_dict) | ||
| self.configs = {"model_reg": self.configs["algorithm"]} |
There was a problem hiding this comment.
This line overwrites the entire configs dictionary with a single key-value pair, discarding all other configuration. It should update the dictionary instead: self.configs['model_reg'] = self.configs['algorithm']
| self.configs = {"model_reg": self.configs["algorithm"]} | |
| self.configs["model_reg"] = self.configs["algorithm"] |
| path_artifact = self._model_path.get_latest_model_artifact_path(run_type) | ||
|
|
||
| self.config["timestamp"] = path_artifact.stem[-15:] | ||
| self.configs = {"timestamp": path_artifact.stem[-15:]} |
There was a problem hiding this comment.
This line overwrites the entire configs dictionary with only the timestamp, losing all other configuration values. It should update the dictionary instead: self.configs['timestamp'] = path_artifact.stem[-15:]
| path_artifact = self._model_path.get_latest_model_artifact_path(run_type) | ||
|
|
||
| self.config["timestamp"] = path_artifact.stem[-15:] | ||
| self.configs = {"timestamp": path_artifact.stem[-15:]} |
There was a problem hiding this comment.
This line overwrites the entire configs dictionary with only the timestamp, losing all other configuration values. It should update the dictionary instead: self.configs['timestamp'] = path_artifact.stem[-15:]
| self.configs = {"timestamp": path_artifact.stem[-15:]} | |
| self.configs["timestamp"] = path_artifact.stem[-15:] |
tests/test_stepshifter_manager.py
Outdated
| stepshifter_manager_hurdle.configs = hurdle_args | ||
|
|
||
| stepshifter_manager_hurdle._get_model(mock_partitioner_dict) | ||
| mock_hurdle_model.assert_called_once_with(stepshifter_manager_hurdle.config, mock_partitioner_dict) |
There was a problem hiding this comment.
The assertion references stepshifter_manager_hurdle.config, but the attribute has been renamed to configs throughout the codebase. This should be stepshifter_manager_hurdle.configs.
tests/test_stepshifter_manager.py
Outdated
| stepshifter_manager.configs = non_hurdle_args | ||
|
|
||
| stepshifter_manager._get_model(mock_partitioner_dict) | ||
| mock_stepshifter_model.assert_called_once_with(stepshifter_manager.config, mock_partitioner_dict) |
There was a problem hiding this comment.
The assertion references stepshifter_manager.config, but the attribute has been renamed to configs throughout the codebase. This should be stepshifter_manager.configs.
| mock_stepshifter_model.assert_called_once_with(stepshifter_manager.config, mock_partitioner_dict) | |
| mock_stepshifter_model.assert_called_once_with(stepshifter_manager.configs, mock_partitioner_dict) |
tests/test_stepshifter_manager.py
Outdated
| artifact_name = None | ||
| stepshifter_manager._evaluate_model_artifact(eval_type, artifact_name) | ||
|
|
||
| assert stepshifter_manager.config["run_type"] == "test_run_type" |
There was a problem hiding this comment.
The assertion references stepshifter_manager.config, but the attribute has been renamed to configs. This should be stepshifter_manager.configs['run_type'].
| assert stepshifter_manager.config["run_type"] == "test_run_type" | |
| assert stepshifter_manager.configs["run_type"] == "test_run_type" |
tests/test_stepshifter_manager.py
Outdated
|
|
||
| assert stepshifter_manager.config["run_type"] == "test_run_type" | ||
| mock_logger.info.assert_called_once_with(f"Using latest (default) run type (test_run_type) specific artifact") | ||
| assert stepshifter_manager.config["timestamp"] == "202401011200000" |
There was a problem hiding this comment.
The assertion references stepshifter_manager.config, but the attribute has been renamed to configs. This should be stepshifter_manager.configs['timestamp'].
| assert stepshifter_manager.config["timestamp"] == "202401011200000" | |
| assert stepshifter_manager.configs["timestamp"] == "202401011200000" |
tests/test_stepshifter_manager.py
Outdated
| assert stepshifter_manager.config["timestamp"] == "202401011200000" | ||
| # mock_read_dataframe.assert_called_once() | ||
| mock_model.predict.assert_called_once_with("test_run_type") | ||
| assert stepshifter_manager.config["run_type"] == "forecasting" |
There was a problem hiding this comment.
The assertion references stepshifter_manager.config, but the attribute has been renamed to configs. This should be stepshifter_manager.configs['run_type'].
tests/test_stepshifter_manager.py
Outdated
| eval_type = "test_eval_type" | ||
| stepshifter_manager._evaluate_sweep(eval_type, mock_model) | ||
|
|
||
| assert stepshifter_manager.config["run_type"] == "test_run_type" |
There was a problem hiding this comment.
The assertion references stepshifter_manager.config, but the attribute has been renamed to configs. This should be stepshifter_manager.configs['run_type'].
| assert stepshifter_manager.config["run_type"] == "test_run_type" | |
| assert stepshifter_manager.configs["run_type"] == "test_run_type" |
1 participant
