3

I was wondering if all self. has to be defined in __init__, for example, i have this code right here:

class Colour:

    def __init__(self, r, g, b):
        self._red = r
        self._green = g
        self._blue = b
        self._rgb = (self._red, self._green, self._blue)


    

    def luminosity(self):
        self._luminosity = 0.5 * ((max(self._red, self._green, self._blue))/255)+((min(self._red, self._green, self._blue))/255)
        return self._luminosity

Am i right to define self.luminosity in the function def luminosity(self) or should i define it in __init__?

Pranav Hosangadi
  • 23,755
  • 7
  • 44
  • 70
  • 1
    According to PEP8 all instance attributes should be defined in `__init__`, so you can do `self._luminosity = self.luminosity()` – DeepSpace Oct 27 '20 at 20:19
  • 1
    Nothing to stop you defining them whenever you want, assuming you defend or control so you don’t get exceptions - but your class will likely be easier to understand if you initialise instance variables in `__init__()` – DisappointedByUnaccountableMod Oct 27 '20 at 20:26

3 Answers3

3

In this case, you don't need to define it, because it's only set and then returned when you could directly return the calculated value from your method!

Additionally, you can simplify the calculation a little, though I am not sure it is really luminosity, as there are a variety of interpretations different to yours

    def luminosity(self):
        return 0.5 * (
            max(self._red, self._green, self._blue) + \
            min(self._red, self._green, self._blue)
        ) / 255

If instead, you were caching the value (which may make sense if you do a more complex calculation or call the luminosity method many times), it would make sense to set it in __init__() and check before calculating (effectively caching the last call)

As @laol suggests, you can also use @property to simplify some of the its use

And finally, you can take advantage of your combined RGB for the calculation

class Colour():

    def __init__(self, r, g, b):
        self._red = r
        self._green = g
        self._blue = b
        self._luminosity = None

    @property
    def rgb(self):
        return (self._red, self._green, self._blue)

    @property
    def luminosity(self):
        if self._luminosity is None:
            self._luminosity = 0.5 * (max(self.rgb) + min(self.rgb)) / 255
        return self._luminosity
c = Colour(128,100,100)
print(c.luminosity)

0.44705882352941173

Extending this even further, setting new values for the color components can set the cached value back to None, triggering re-calculation on the next call (rather than immediately, saving some calculation if many changes are made before the value is wanted), but this is left as an exercise to the reader

ti7
  • 16,375
  • 6
  • 40
  • 68
2

I suggest to define it as a property:

 @property
def luminosity(self):
    return 0.5 * ((max(self._red, self._green, self._blue))/255)+((min(self._red, self._green, self._blue))/255)

By this you can directly return it from any Colour c by

c.luminosity
laol
  • 414
  • 6
  • 9
2

No, instance variables do not need to be defined in __init__. Instance variables are completely dynamic and can be added any time either in a method or outside of the object (see note). However, if you don't define them, you have created an object access protocol that needs to be managed. Suppose another method is added:

def half_luminosity(self):
    return self._luminosity/2

It is an error to call it before luminosity. This code will raise AttributeError if its called at the wrong time. You could assign self._luminosity = None in __init__ and check it

def half_luminosity(self):
    if self._luminosity is None:
        raise ValueError("Attempt to use luminosity before set")

but that's not much different than

def half_luminosity(self):
    if not hasattr(self, '_luminosity'):
        raise ValueError("Attempt to use luminosity before set")

If you have a class that is setup in more than one step, either way will do. PEP8 favors the first because its easier for a futurer reader to see what's going on.

NOTE: Classes that use __slots__ or one of the getattr methods can change the rules as can C extensions.

tdelaney
  • 73,364
  • 6
  • 83
  • 116