Skip to content

Conversation

merschformann
Copy link
Member

@merschformann merschformann commented May 29, 2025

Description

Caution

This is WIP and can't be merged as it is incomplete and potentially not the solution to go with.

Resolves ENG-6089

Copy link
Contributor

@Copilot 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 fixes sklearn pickling issues by updating the nextmv dependency version and adding default values to LinearRegression options. The changes ensure compatibility with newer versions of the nextmv library while making the sklearn integration more robust.

  • Updates nextmv dependency from >=0.25.0 to >=0.28.1 across packages
  • Adds missing default values to LinearRegression option parameters
  • Introduces comprehensive test coverage for model pickling functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
nextmv-scikit-learn/tests/test_pickle.py Adds new test file for validating model pickling functionality
nextmv-scikit-learn/pyproject.toml Updates nextmv dependency version to >=0.28.1
nextmv-scikit-learn/nextmv_sklearn/linear_model/options.py Adds default values to LinearRegression parameters
nextmv-scikit-learn/nextmv_sklearn/about.py Bumps version to v0.3.1.dev1
nextmv-gurobipy/pyproject.toml Updates nextmv dependency version to >=0.28.1

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

def solve(self, input: nextmv.Input) -> nextmv.Output:
if input.options.mode == "linear":
model = LinearRegression(input.options)
_ = model
Copy link
Preview

Copilot AI Sep 15, 2025

Choose a reason for hiding this comment

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

The variable assignment _ = model serves no purpose and should be removed. If the intent is to instantiate the model for testing, use the model directly in the return statement or assign it to a meaningful variable name.

Suggested change
_ = model

Copilot uses AI. Check for mistakes.

@merschformann
Copy link
Member Author

Caution

This is WIP and can't be merged as it is incomplete and potentially not the solution to go with.

Copy link
Contributor

@Copilot 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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

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.

2 participants