8
\$\begingroup\$

I just filled in the first assignment on course which involves learning Python. The assignment was to make vector class which supports scalar multiplication through operator overloading.

The instructions included these constains:

  • There will be one class called MyVector
  • Nothing will be imported in file within that file which contains the class
  • The class will accept one argument in constructor, which must be list of numbers
  • The class will have two methods:
    • get_vector which returns list of numbers of the vector
    • __mul__ (overloaded * operator) which performs scalar multiplication of two vectors

This is what I have sent:

class MyVector:
    """Vector class"""
    coordinates = None
    def __init__(self, coordinates):
        if not isinstance(coordinates, list):
            raise TypeError("Coordinates of vector must me single dimensional array.")
        self.coordinates = coordinates
    def f(self):
        return 'hello world'
    def get_vector(self):
        return self.coordinates
    def dimensions(self):
        return len(self.coordinates)
    def __mul__(self,other):
        if other.dimensions() != self.dimensions():
            raise ValueError("Number of dimensions of multiplied vectors must be equal.")
        tmp = 0
        for index in range(self.dimensions()):
            tmp += self.coordinates[index] * other.coordinates[index]
        return tmp

''' Just a testing section recommended in the assignment '''
if __name__ == "__main__":
    vec1 = MyVector([1,2,5,5,5]) # vektory mohou byt i jine dimenze nez 3!
    vec2 = MyVector([1,2,5,5,5]) 
    print(vec1.get_vector()) # Test getting the list of items
    dot_product = vec1*vec2  # Multiplication test
    print(dot_product)

The homework was OK, but the validation system is bragging that their implementation is faster:

Message: module file vectors.py found
Result ok
Your elapsed time for 10,000 trials, vectors length 300: 0.675 seconds
Our elapsed time for the same setting: 0.383 seconds

Points: 2 out of 2

I'll repeat the relevant section of code here:

    def __mul__(self,other):
        if other.dimensions() != self.dimensions():
            raise ValueError("Number of dimensions of multiplied vectors must be equal.")
        tmp = 0
        for index in range(self.dimensions()):
            tmp += self.coordinates[index] * other.coordinates[index]
        return tmp

As you can see, as a seasoned C++ programmer I took naïve approach with indexed for loop. I know that Python has some famous tools to deal with array operation.

Is there some really pythonic way to loop over those two arrays and multiply their elements? Note that primarily I'm looking for pythonic way (since I'm learning Python), optimalization is secondary. I got my 2 points for homework, I'm not submiting it again.

\$\endgroup\$
  • \$\begingroup\$ I’ve added the python-3.x tag as regard to the print statement, feel free to correct it if I’m wrong. \$\endgroup\$ – Mathias Ettinger Oct 4 '16 at 13:57
  • \$\begingroup\$ "Scalar multiplication of two vectors" is a terrible phrase! \$\endgroup\$ – Gareth Rees Oct 4 '16 at 16:03
  • 1
    \$\begingroup\$ @GarethRees Well, I'm not native English speaker. What's the correct phrasing? \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 17:09
  • 1
    \$\begingroup\$ @TomášZato: "Scalar multiplication" normally means multiplication of a vector by a scalar; the operation you've been asked to code is sometimes called the "scalar product" but most people call it the "dot product" to avoid confusion. \$\endgroup\$ – Gareth Rees Oct 4 '16 at 18:06
  • 1
    \$\begingroup\$ @GarethRees I edited the title. \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 18:17
14
\$\begingroup\$

The most "pythonic" way of doing this makes use of the sum and zip built-ins:

def __mul__(self,other):
    if other.dimensions() != self.dimensions():
        raise ValueError("Number of dimensions of multiplied vectors must be equal.")
    return sum(a * b for a, b in zip(self.coordinates, other.coordinates))

I suggest making use of the REPL (interactive shell) to learn about them, here is an example REPL session that you can read explaining zip and sum:

>>> x = [1, 4, 8]
>>> sum(x)
13
>>> y = [6, 1, 0]
>>> zip(x, y)
<zip object at 0x7f51da3db5c8>
>>> list(_) # `_` means previous value
[(1, 6), (4, 1), (8, 0)]
>>> help(zip)
Help on class zip in module builtins:

class zip(object)
 |  zip(iter1 [,iter2 [...]]) --> zip object
 |  
 |  Return a zip object whose .__next__() method returns a tuple where
 |  the i-th element comes from the i-th iterable argument.  The .__next__()
 |  method continues until the shortest iterable in the argument sequence
 |  is exhausted and then it raises StopIteration.
 |  
 |  Methods defined here:
 |  
 |  __getattribute__(self, name, /)
 |      Return getattr(self, name).
 |  
 |  __iter__(self, /)
 |      Implement iter(self).
 |  
 |  __new__(*args, **kwargs) from builtins.type
 |      Create and return a new object.  See help(type) for accurate signature.
 |  
 |  __next__(self, /)
 |      Implement next(self).
 |  
 |  __reduce__(...)
 |      Return state information for pickling.

>>> help(sum)
Help on built-in function sum in module builtins:

sum(...)
    sum(iterable[, start]) -> value

    Return the sum of an iterable of numbers (NOT strings) plus the value
    of parameter 'start' (which defaults to 0).  When the iterable is
    empty, return start.

>>> zip( (1,2,3), "abc" )
<zip object at 0x7f51dba79648>
>>> list(_)
[(1, 'a'), (2, 'b'), (3, 'c')]

Other points

  • Tests should be automatic and NOT rely on you manually checking that wha is printed is right. This manual method will become too time consuming very soon.

  • f makes no sense (probably it is a leftover from a previous assignment). Please remove it.

  • get methods are discouraged in Python, accessing the items directly is very simple anyway.

  • Use @property to give convenient call sintax in dimensions(self) or remove this property completely as you can just call len(vector.coordinates) very easily.

\$\endgroup\$
  • 1
    \$\begingroup\$ Actually I am against unnecessary get methods in any language, but this was part of the assignment requirements. As you can see in the code, I demonstratively accessed the property directly, ignoring the getter. And thanks for mentioning @property, I'm going to see how it works. \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 14:03
  • 1
    \$\begingroup\$ @TomášZato I still have one question for you: what does coordinates = None at the start of the class do? I think you can just remove it. \$\endgroup\$ – Caridorc Oct 4 '16 at 14:05
  • \$\begingroup\$ I like to explicitly specify what properties can some object have - even in prototype based languages. I do the same in JavaScript, eg. Vector.prototype.coordinates = null. It also creates natural space where comments with description can fit. \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 14:11
  • \$\begingroup\$ @TomášZato please note that that first assignment creates a value that is statically shared by all vector classes if not over-written (but in this case it is always over-written). Also do not use code that does nothing as documentation, if need be, use actual documentation. \$\endgroup\$ – Caridorc Oct 4 '16 at 14:13
  • \$\begingroup\$ I know that, it's same in javascript. That's why I use an invalid value, instead of a list for example. Seeing the names of class properties really helps me to work faster... But yeah, it would have about the same effect if it was inside comment. \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 14:17
10
\$\begingroup\$
  • You should use blank lines more often, it will improve readability by making logical sections of code more visible
  • You may want to rename dimensions to __len__ which is more natural for a python container
  • You could have an __iter__ method that allow you direct iteration over the vector:

    def __iter__(self):
        return iter(self.coordinates)
    

    So you can use

    >>> v = MyVector(1, 2, 5, 5, 5)
    >>> for e in v:
    ...     print(e)
    ...
    1
    2
    5
    5
    5
    

    instead of:

    >>> for e in v.get_vector():
    

All of this lead me to think that you could inherit from a standard container to have all this "for free". For instance:

class MyVector(tuple):
    """..."""

    def get_vector(self):
        return list(self)

    def __mul__(self, other):
        if len(self) != len(other):
            raise ValueError('...')
        # Uses improvements from @Caridorc answer
        return sum(a * b for a, b in zip(self, other))

So you can directly call len, or iterate over, or get a value at a given index on your vectors:

>>> v1 = MyVector([1, 2, 5, 5, 5])
>>> v2 = MyVector(3*x + 2 for x in v1)
>>> v1
(1, 2, 5, 5, 5)
>>> v2
(5, 8, 17, 17, 17)
>>> v1 * v1
80
>>> v1 * v2
276
>>> v2 * v2
956
\$\endgroup\$
  • 1
    \$\begingroup\$ Wow, thanks. Asking this question was definitely worth it! :) \$\endgroup\$ – Tomáš Zato Oct 4 '16 at 14:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.