237

I split up my class constructor by letting it call multiple functions, like this:

class Wizard:
    def __init__(self, argv):
        self.parse_arguments(argv)
        self.wave_wand() # declaration omitted

    def parse_arguments(self, argv):
        if self.has_correct_argument_count(argv):
            self.name = argv[0]
            self.magic_ability = argv[1]
        else:
            raise InvalidArgumentsException() # declaration omitted

# ... irrelevant functions omitted

While my interpreter happily runs my code, Pylint has a complaint:

Instance attribute attribute_name defined outside __init__

A cursory Google search is currently fruitless. Keeping all constructor logic in __init__ seems unorganized, and turning off the Pylint warning also seems hack-ish.

What is a/the Pythonic way to resolve this problem?

Improve this question
9
  • 9
    The warning just says what it says. I thinks it violates the POLS if you initialize instance variables de-facto outside the constructor. Try to inline parse_arguments or use the return values of the function in __init__ to initialize the variables and pylint will be happy, I guess. Commented Oct 10, 2013 at 0:10
  • 1
    "the accepted answer was to set each [attribute] to None and then you can access them from [methods] without an issue" - yes, that. Or disable/ignore the warning. It would be helpful to actually link that post. Commented Jan 3, 2024 at 12:16
  • 2
    You can disable this Pylint warning with # pylint: disable=attribute-defined-outside-init comment in __init__ (make sure to reenable it later.) Commented Jan 3, 2024 at 12:25
  • 1
    Does the warning go away if you add speed_value: float in the class Motor scope? (Not in __init__) Commented Jan 3, 2024 at 12:30
  • 1
    @DiBosco The comment about disabling warnings is the correct solution, not merely a hint. Your current code is correct; it's pylint that's wrong. The fact is that pylint makes many highly opinionated assumptions that will often conflict with your well-reasoned intentions. So you must configure it in whatever way is most appropriate for a given context. The disabling comment is almost self-documenting (esp. if it precedes a well-named method), so it should be clear to anyone reading the code what the intention is. (And note re-enabling is unnecessary as it only applies to the current block). Commented Jan 3, 2024 at 14:21

7 Answers 7

Reset to default
249

The idea behind this message is for the sake of readability. We expect to find all the attributes an instance may have by reading its __init__ method.

You may still want to split initialization into other methods though. In such case, you can simply assign attributes to None (with a bit of documentation) in the __init__ then call the sub-initialization methods.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

is there a reference to why "We expect to find all the attributes an instance may have by reading its init method." It makes sense, but curious.
@MericOzcan either setting a bunch of attributes to None and then setting them later in a parse_args function OR returning a short tuple from parse_args is OK. ideally, parse_args should be testable without needing a wizard instance.
Is this still relevant when the attributes are defined in a parent. It seems a big overhead for readability when super is called indicating members can be defined there?
But when I do that (initialize to None the attributes), mypy complains, any suggestions on how to fix both issues at the same time?
I would LOVE to keep all instance attributes inside of init, as this is what I'm used to with Java. However, I just discovered that when extending Python classes, the parent can see and operate on child instance variables EVEN when not passed to the parent through Super. This makes extending some classes very difficult, because the parent class will operate on the child specific class attributes in ways that are undesireable. This is why I have ignored this particular warning myself. I also like your idea about using None and initializing later!
|
43

Just return a tuple from parse_arguments() and unpack into attributes inside __init__ as needed.

Also, I would recommend that you use Exceptions in lieu of using exit(1). You get tracebacks, your code is reusable, etc.

class Wizard:
    def __init__(self, argv):
        self.name,self.magic_ability = self.parse_arguments(argv)

    def parse_arguments(self, argv):
        assert len(argv) == 2
        return argv[0],argv[1]
Improve this answer

4 Comments

Since this is a relatively simple program and the main code will be constructing a single Wizard (the main class), I figured that exceptions were overkill.
@StevenLiao that's all well and good, but it's still more readable, fewer lines of code, and develops good habits for the future. Up to you.
OK, but what if you only want to have those attributes added by calling parse_arguments to the object in special circumstances? In this case setting the attributes equal to None is a better solution.
also what if you have more than two members it gets uglier as that number grows
12

Define the fields as annotations in the class body to tell PyCharm (and your readers) that you intend for these to exist (with the type given).

class Motor:
    speed_value: float
    hours_value: float

    def __init__(self):
        self.reset_all_values()

    def reset_all_values(self):
        self.speed_value = DEFAULT_SPEED_VALUE
        self.hours_value = DEFAULT_HOURS_VALUE
Improve this answer

7 Comments

Thanks for this suggestion. This looks more like a C/C++ way of declaring a type of variable. As a matter of interest does this have the added bonus that Python would flag up a warning/error if someone tried to pass the wrong type of variable to a function that is an accessor to modify one of these variables?
Python wouldn't (since type annotations are not enforced at runtime), but PyCharm and other IDEs and likely type checkers like Mypy and Pyright would.
OK thanks. speed_value: float or indeed speed_value: int which is what it really is, doesn't stop Pycharm flagging up the warning
It does for me. PyCharm 2023.3 (Professional Edition)
I'm on the community edition, maybe that's the issue.
|
3

The best practice to solve this question is you need to build the parameter in Init part first, Then adjust it in the Def

class MainApplication(tk.Frame):
    def __init__(self, master):
        self.master = master
        tk.Frame.__init__(self, self.master)
        self.settingsFrame = None
        self.create_widgets(master)

    def create_widgets(self, master):
        # frame Container
        self.settingsFrame = tk.Frame(self.master, width=500, height=30, bg='white')
Improve this answer

2 Comments

This is just a lower-quality rewording of part of the top answer, posted 7 years earlier stackoverflow.com/a/19292653/5067311.
@AndrasDeak--СлаваУкраїні depreciating others answer doesn't make you become looks more professional.
2

Although the definition of instance variables outside __init__ isn't recommended in general, there are rare cases in which it is natural. For example, when you have a parent class that defines several variables that its child classes won't use, and whose definition will make its child waste time or resources, or will be simply unaesthetic.

One possible solution to this is using an init-extension function that each child class may override, and in this function use function setattr in order to define the class-unique instance variables. Maybe this is not too aesthetic as well, but it eliminates the linting warning discussed here.

Improve this answer

1 Comment

If child classes win't use some parent class members then this is OOP :). Or you could rewrite parent class constructor to get internal variable value as its argument.
0

For each attribute you want to set via function, call the function from the init. For example, the following works for me to set the attribute ascii_txt...

def __init__(self, raw_file=None, fingerprint=None):
    self.raw_file = raw_file
    self.ascii_txt = self.convert_resume_to_ascii()

def convert_resume_to_ascii(self):
    ret_val = self.raw_file.upper()
    return ret_val
Improve this answer

Comments

-3

If you are using Python 3, you can try

class Wizard:
    def __init__(self, argv):
        self.name: str = str()
        self.magic_ability: str = str()
        self.parse_arguments(argv)
        self.wave_wand() # declaration omitted

    def parse_arguments(self, argv):
        if self.has_correct_argument_count(argv):
            self.name = argv[0]
            self.magic_ability = argv[1]
        else:
            raise InvalidArgumentsException() # declaration omitted

# ... irrelevant functions omitted

Although not as pythonic as the accepted answer, but it should get away the Pylint alert.

And if you don't concern about type and don't want to create a new object with object() use:

class Wizard:
    def __init__(self, argv):
        self.name = type(None)()
        # ...

As None will cause type not match error.

Improve this answer

3 Comments

am using pyton3 and pycharm, and this solution has the merit to avoid another warning caused when you declare self.my_string:str = None => "Expected type 'str' got None instead. Am curious if str() takes more memory than None, and why this solution is less pythonic if it avoids an IDE warning...
I checked onto the memory usage >>> import sys >>> print(sys.getsizeof(None)) 16 >>> print(sys.getsizeof(str())) 49 And the whether if my solution is pythonic I could not say. But I absolutely favour declaring variables on init as it makes the code easier to read.
why ` type(None)()` instead of None?

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.