Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

ENH Show a more informative error when accessing an attribute #31066

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
Loading
from
Draft

ENH Show a more informative error when accessing an attribute #31066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
827d6cb
Show a more informative error when accessing an attribute without cal…
thomasjpfan Mar 25, 2025
42b2b9c
Add unit tests
thomasjpfan Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion 10 sklearn/linear_model/_ridge.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
# Authors: The scikit-learn developers
# SPDX-License-Identifier: BSD-3-Clause


import numbers
import warnings
from abc import ABCMeta, abstractmethod
Expand Down Expand Up @@ -39,6 +38,7 @@
get_namespace,
get_namespace_and_device,
)
from ..utils._attribute_validation import FittedAttribute
from ..utils._param_validation import Hidden, Interval, StrOptions, validate_params
from ..utils.extmath import row_norms, safe_sparse_dot
from ..utils.fixes import _sparse_linalg_cg
Expand Down Expand Up @@ -2298,6 +2298,14 @@ def __sklearn_tags__(self):


class _BaseRidgeCV(LinearModel):
cv_results_ = FittedAttribute("store_cv_results=True and cv=None")
coef_ = FittedAttribute()
intercept_ = FittedAttribute()
alpha_ = FittedAttribute()
best_score_ = FittedAttribute()
n_features_in_ = FittedAttribute()
feature_names_in_ = FittedAttribute()

_parameter_constraints: dict = {
"alphas": ["array-like", Interval(Real, 0, None, closed="neither")],
"fit_intercept": ["boolean"],
Expand Down
22 changes: 22 additions & 0 deletions 22 sklearn/utils/_attribute_validation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Authors: The scikit-learn developers
# SPDX-License-Identifier: BSD-3-Clause

from dataclasses import dataclass, field


@dataclass
Copy link
Member

Choose a reason for hiding this comment

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

For my education, why use a dataclass instead of a plain class that implements __get__ and a constructor? Taste?

Copy link
Member

Choose a reason for hiding this comment

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

I guess the reason is NOT to have to implement the constructor 😅 Syntactic sugars are there to reduce boilerplate, that's a good enough of a reason for me to use them.

class FittedAttribute:
"""Descriptor to raise a better error when attribute is not defined."""

condition: str = ""
name: str = field(init=False)

def __get__(self, instance, owner):
if self.condition:
msg = f"Call `fit` with {self.condition} to define '{self.name}'"
else:
msg = f"Call `fit` to define '{self.name}'"
raise AttributeError(msg)

def __set_name__(self, owner, name):
self.name = name
28 changes: 28 additions & 0 deletions 28 sklearn/utils/tests/test_attribute.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import pytest

from sklearn.utils._attribute_validation import FittedAttribute


class MyObj:
coef_ = FittedAttribute()
Copy link
Member

Choose a reason for hiding this comment

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

Layering on the magic: is it possible to hide these attributes from tab completion when they haven't been fit yet?

Right now m = MyObj() followed by m.<tab> (in IPython) lists all the attributes which is different from the current behaviour and somehow not great IMHO.

Copy link
Member

Choose a reason for hiding this comment

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

To answer my own question:

class MyObj:
    def __getattr__(self, name):
        if name == "coef_":  # could be `if name in self._list_of_potential_names`
            raise AttributeError(f"{self.__class__.__name__} object has no attribute {name}!")
    def fit(self):
        self.coef_ = 123

This way m = MyObj() followed by m.<tab> doesn't show coef_ as a possible completion

cv_results_ = FittedAttribute("cv=None")

def fit(self):
self.coef_ = 123
self.cv_results_ = [1, 2, 3]


def test_attribute_error():
my_obj = MyObj()

msg = "Call `fit` to define 'coef_'"
with pytest.raises(AttributeError, match=msg):
my_obj.coef_

msg = "Call `fit` with cv=None to define 'cv_results_'"
with pytest.raises(AttributeError, match=msg):
my_obj.cv_results_

my_obj.fit()
assert my_obj.coef_ == 123
assert my_obj.cv_results_ == [1, 2, 3]
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.