ENH Show a more informative error when accessing an attribute #31066
Conversation
|
What about |
|
How about |
|
|
||
|
|
||
| class MyObj: | ||
| coef_ = FittedAttribute() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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_ = 123This way m = MyObj() followed by m.<tab> doesn't show coef_ as a possible completion
| from dataclasses import dataclass, field | ||
|
|
||
|
|
||
| @dataclass |
There was a problem hiding this comment.
For my education, why use a dataclass instead of a plain class that implements __get__ and a constructor? Taste?
There was a problem hiding this comment.
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.
|
As a point of reference: in cuml descriptors are used for fitted attributes in order to handle conversion of that attribute to the right kind of array (numpy, cupy, ..) depending on the context/config in which it is accessed. I am not a fan of it as it causes a lot of trouble and complexity. I don't know if they are implemented in a particular way that causes this pain or if it is a general feature. I'd love to be bale to remove the need for descriptors in cuml. This PR looks way less complex and harmless, but I thought I'd leave my observation of dealing with a library that extensively uses descriptors |
Reference Issues/PRs
Closes #31010
What does this implement/fix? Explain your changes.
This PR implements a descriptor to raise a nicer error.
Any other comments?
I'm not sure if I like it, but it's the "least magical".
I want the pattern to be "the class that sets the attributes should define
FittedAttribute". In this case,_BaseRidgeCVdefines them all, so it'll set them.