Skip to content

Use latest views-pipeline-core package#44

Merged
xiaolong0728 merged 16 commits intomainfrom
development
Dec 11, 2025
Merged

Use latest views-pipeline-core package#44
xiaolong0728 merged 16 commits intomainfrom
development

Conversation

@xiaolong0728
Copy link
Collaborator

  1. Support classification models using the latest Darts library and update classes accordingly.
  2. Update classes to adapt to the latest views-pipeline-core

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the views-stepshifter package to use the latest views-pipeline-core package and Darts library (v0.38.0). Key changes include migrating to native Darts classification models, applying log transformations to targets, and updating the configuration attribute name from config to configs throughout the codebase.

  • Migrates custom model implementations to use native Darts models (XGBClassifierModel, LightGBMClassifierModel)
  • Adds log/exp transformations for target values in predictions
  • Updates configuration attribute naming convention across the manager classes

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
views_stepshifter/models/stepshifter.py Removes unused import, updates deprecated method call, adds log transformations, comments out CUDA support
views_stepshifter/models/hurdle_model.py Switches from custom to native Darts classification models, comments out CUDA configuration
views_stepshifter/models/darts_model.py Replaces extensive custom model implementations with simple wrapper classes inheriting from Darts models
views_stepshifter/models/init.py Removes exports for models now provided by Darts library
views_stepshifter/manager/stepshifter_manager.py Renames config attribute to configs throughout the manager class
tests/test_stepshifter_manager.py Updates test fixtures and assertions to match new configuration attribute naming
tests/test_hurdle_model.py Updates test configuration to use supported model names
pyproject.toml Bumps version to 1.1.0, updates Darts dependency to ^0.38.0, adds xgboost ^3.0.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"]}
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment overwrites the entire configs dictionary with a single key, losing all other configuration values. Should merge this key into the existing dictionary instead, e.g., self.configs['model_reg'] = self.configs['algorithm'] or create a new dict: self.configs = {**self.configs, 'model_reg': self.configs['algorithm']}.

Suggested change
self.configs = {"model_reg": self.configs["algorithm"]}
self.configs["model_reg"] = self.configs["algorithm"]

Copilot uses AI. Check for mistakes.
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:]}
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment overwrites the entire configs dictionary, discarding all existing configuration. Should merge instead: self.configs['timestamp'] = path_artifact.stem[-15:] or self.configs = {**self.configs, 'timestamp': path_artifact.stem[-15:]}.

Copilot uses AI. Check for mistakes.

self.config["timestamp"] = path_artifact.stem[-15:]

self.configs = {"timestamp": path_artifact.stem[-15:]}
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assignment overwrites the entire configs dictionary, discarding all existing configuration. Should merge instead: self.configs['timestamp'] = path_artifact.stem[-15:] or self.configs = {**self.configs, 'timestamp': path_artifact.stem[-15:]}.

Suggested change
self.configs = {"timestamp": path_artifact.stem[-15:]}
self.configs['timestamp'] = path_artifact.stem[-15:]

Copilot uses AI. Check for mistakes.
mock_path.model_dir = "/path/to/models/test_model"
mock_path.target = "model"
mock_path.artifacts = Path("/path/to/artifacts")
mock_path.get_latest_model_artifact_path.return_value = Path("test_model_202401011_200000")
Copy link

Copilot AI Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected spelling in path from 'test_model_202401011_200000' to 'test_model_20240101_200000' - appears to have extra '1' digit.

Suggested change
mock_path.get_latest_model_artifact_path.return_value = Path("test_model_202401011_200000")
mock_path.get_latest_model_artifact_path.return_value = Path("test_model_20240101_200000")

Copilot uses AI. Check for mistakes.
@xiaolong0728 xiaolong0728 merged commit 77dc017 into main Dec 11, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant