Skip to content

[BUG] preliminary fix for incorrect setting of _pretrained_attrs in case __init__ already sets fitted attrs#10535

Merged
fkiraly merged 7 commits into
mainfrom
pretrained-fix
Jul 11, 2026
Merged

[BUG] preliminary fix for incorrect setting of _pretrained_attrs in case __init__ already sets fitted attrs#10535
fkiraly merged 7 commits into
mainfrom
pretrained-fix

Conversation

@fkiraly

@fkiraly fkiraly commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

Bug #10531 is triggered in case __init__ already sets fitted attrs, e.g., for initialization (the diagnosis in the issue is incorrect, see discussion below).

This PR adds a preliminary fix: only those attrs are added to self._pretrained_attrs that do not already exist once pretrain is entered.

A test for regression is also added.

For a final fix, an API design discussion needs to be reopened first.

@fkiraly fkiraly added module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting bugfix Fixes a known bug or removes unintended behavior labels Jul 11, 2026
@fkiraly
fkiraly marked this pull request as ready for review July 11, 2026 16:07
Copilot AI review requested due to automatic review settings July 11, 2026 16:07

Copilot AI left a comment

Copy link
Copy Markdown

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 implements a preliminary fix to BaseForecaster.pretrain so that _pretrained_attrs only tracks attributes that first appear during pretrain, avoiding misclassification of fitted-style attributes that were already present when entering pretrain (e.g., attributes initialized in __init__). It also adds a regression test targeting the reported behavior in #10531.

Changes:

  • Capture public trailing-underscore attributes present at pretrain entry and exclude them from _pretrained_attrs.
  • Add a regression test ensuring NaiveForecaster.pretrain does not register pre-existing attrs as pretrained params.

Reviewed changes

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

File Description
sktime/forecasting/base/_base.py Excludes pre-existing public *_ attributes from being added to _pretrained_attrs during pretrain.
sktime/forecasting/base/tests/test_base.py Adds regression coverage to ensure pretrain doesn’t misclassify pre-existing attrs as pretrained.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sktime/forecasting/base/_base.py Outdated
Comment thread sktime/forecasting/base/tests/test_base.py Outdated
@fkiraly
fkiraly merged commit 5bb3324 into main Jul 11, 2026
81 of 82 checks passed
@fkiraly
fkiraly deleted the pretrained-fix branch July 11, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a known bug or removes unintended behavior module:forecasting forecasting module: forecasting, incl probabilistic and hierarchical forecasting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants