slac207 / cs207project

MIT License
0 stars 4 forks source link

Add operations of 2 variables (project 4 page, Part 2) #14

Closed brennan-ae closed 7 years ago

brennan-ae commented 7 years ago

Part 2: Add to these methods(again well tested):

infix addition, subtraction, equality and multiplication. Here you must check that the lengths are equal and that the time domains are the same for the case of the operations on a TimeSeries (the latter implies the former). Return a ValueError in case this fails:

ValueError(str(self)+' and '+str(rhs)+' must have the same time points')

Let these be elementwise operations, as we might expect from a numpy array-like thing. As before, handle the case of a constant. - unary __abs__, __bool__, __neg__, and __pos__ with the same semantics as the Vector class we saw in lecture.

A question that might arise is what to do if we add numpy arrays or regular python lists to a Timeseries. These should fail with raise NotImplemented as we dont have time associated. An option might have been to associate the array with the time indexing of the other array, but this is making too many assumptions: the user can do this explicitly.

You will probably have to catch another exception for this to happen.

Put this code into your project repo.

brennan-ae commented 7 years ago

When adding to numpy arrays, numpy performs an __radd__ and returns a numpy array using instance._values as a numpy array. Is this behavior we want? If not, we will need to explicitly reject it.

brennan-ae commented 7 years ago

@sanoke @jsm09 @cocochrane

When we add with a numpy array, we probably want to fail with NotImplemented.

However, if we do that, numpy will say "but I know how to add with this thing because it has an iter method." So it adds to the TS._values and returns a numpy array (using its own radd).

Does anyone know how to prevent this?

I could explicitly check for numpy arrays and return a different error than NotImplemented, but this isn't consistent.

Thanks!

jsm09 commented 7 years ago

Can we close this one? @brennan-ae @sanoke @cocochrane