Calling a function within a method

Question

I am writing a class to handle some data and operations on the data:

class AstroData:
 def __init__(self): 
 self.positions = numpy.array ([])
 self.positions_coord_system = ""
 def change_to_rad (self):
 self.positions = deg_to_rad(self.positions)

then I want to do something with the data, say change the units from degrees to radians (I know it is a lame example, but it illustrates my question; another would be to change coordinate systems). I have a function to change degrees to radians (or to change to a coordinate system):

def deg_to_rad(degree_vals):
 return numpy.deg2rad (degree_vals)

My question is: should I be calling a function in the method, or should the function be defined within the method, e.g.

class AstroData:
 def __init__(self): 
 self.positions = numpy.array ([])
 self.positions_coord_system = ""
 def change_to_rad (self):
 self.positions = numpy.deg2rad (self.positions)

I would like to know which is the better quality solution and the proper way to do this. Of course, for more complicated functions it would be a considerable block of code which I would have to put inside the class.

• RomanPerekhrest: you may use your wrapper function deg_to_rad to encapsulate a specific numpy.deg2rad call and perhaps, with a possibility to extend that custom function and use in various places in application
• juanpa.arrivillaga: calling functions in methods it totally normal. You haven't actually defined your function in the method, you could do that, but it would be unecessarilyn inefficient in this case (only when you actually need to create a new function on each invocation of the method, which you dont')
• Bakuriu: What's the point of adding boilerplate code? IMHO writing wrapper is fine only if you want to avoid a strong dependency on that specific numpy function. For example, maybe now your "backend" is numpy but in the future you want to support an other vectorization library, or if you already know that those functions will be more complex soon.
• Collin: You didn't ask this, but I would pick one, consistent internal representation of the angles in this class (either radians or degrees) and store that in self.positions. Then, if someone wants the other representation, convert it on the fly with a as_degrees() or as_radians() method which returns the representation you want. As it stands, it's not possible to tell what the representation is, everyone has to know whether you've called change_to_rad() or not.

Answer 1

I would answer, it depends, if you will use the function deg_to_rad in addition to change_to_rad in other parts of your code, the correct is to put that part of the code in the function (deg_to_rad). If it is a only one call, and it is very short, its ok to not create the function. But even thou, is better to separate functionality, so if you have to make a change, you will look in deg_to_rad and not in change_to_rad code.

Answer 2

I think this depends on the situation. In this case I would make sure that what is stored as self.positions is always consistent, that is, either always degrees or always radians. I wouldn't arbitrarily switch them.

I would then have two methods on the object, one called get_postitons_as_deg and another called get_positions_as_rad, which converts the values and returns them (leaving self.positions untouched).

The caller can then decide how it'll want those values.

Answer 3

This is somewhat of an opinion-based question, but in situations like this I think it's useful to keep in mind the advice of PEP 20:

Explicit is better than implicit.

By defining deg_to_rad() as a method in your class, you are telling the world that "this is something that my class is supposed to do". Even if it's just forwarding that call to a specialized numpy function that does something, it makes a statement to anyone using your code that "this is something this class is designed to do".

That said,

Simple is better than complex.
...
Although practicality beats purity.
...
There should be one-- and preferably only one --obvious way to do it.

What it looks like to me is that, if you were to include the method deg_to_rad() in your class AstroData, it would be one random static method in a class that's meant to be a data structure. If users want the functionality of converting degrees to radians, in general, a solution already exists for them (it's the one you're using).

---

One of the more important aspects of your code, especially if you're creating a data structure to be used across a program, is consistency. The way you're currently doing things, you can have the same data structure holding either one of two different types of data. If someone tries using your data structure, they need to know what type of data it's holding in advance, or else they're going to see behavior they don't want.

Explicit is better than implicit
...
In the face of ambiguity, refuse the temptation to guess.

As other answers have implied, a better way to handle this type of implementation question is to always store data the same way, and only change the format when the caller decides how it wants those. Instead of having a method that changes the format in which data is stored, have methods that change the format in which data is returned:

class AstroData:
 def __init__(self): 
 self.positions = numpy.array ([])
 self.positions_coord_system = ""
 def get_positions_in_degrees(self):
 return self.positions
 def get_positions_in_radians(self):
 return numpy.deg_to_rad(self.positions) # without modifying self.positions

You can, of course, use helper methods when necessary, but ideally everything should be hidden from the user unless it explicitly matters to the user. You can also play around with the names of functions, whatever makes accessing these things the most clear and straightforward - but don't change the format of your data storage while you're in the middle of using it.

• John: Thank you for the nice write-up! But if I do it s you suggest, isn't it better to have get_positions_in_radians() as a standalone function outside of the class then?
• John: I could also include a flag in the class which tells me if the data is in degrees or radians, right? And change it accordingly.
• Green Cloak Guy: You could have a flag, yes, but it would be more straightforward to simply have a different data type for degrees vs radians, and maybe make a method on either class that returns an instance of the other. Again, "explicit is better than implicit", and the clearer the functionality is to the user, the better. You wouldn't want get_positions_in_radians() to be outside the class, because it's still referring to (and presenting) data stored inside the class.
• John: Gotcha! So would a different data type for degrees vs radians mean creating another class for radians then? If not how would it be best to implement it?
• Green Cloak Guy: Presumably, yes, like AstroDataDegrees and AstroDataRadians. The former has a .as_radians() method that returns an AstroDataRadians object with equivalent data, while the latter has a .as_degrees() method that does the reverse