powellb / seapy

State Estimation and Analysis in Python
MIT License
28 stars 21 forks source link

minor bug fix in tide #30

Closed dalepartridge closed 8 years ago

dalepartridge commented 8 years ago

Fix bug during casting to complex numbers when using minor values

powellb commented 8 years ago

This is not the ideal way to create a complex array. Better would be:

ts = np.zeros(len(times))
if tide_minor:
    ts.dtype = np.complex

Normally, you don't want to do that. To be safe, you can do ts = ts.astype(np.complex, copy=False). However, for this case, we just created zeros (and there is a default conversion between 0 float and 0 complex).

dalepartridge commented 8 years ago

Okay, modified the pull request

On Oct 12, 2016, at 8:03 PM, Brian Powell notifications@github.com wrote:

This is not the ideal way to create a complex array. Better would be:

ts = np.zeros(len(times)) if tide_minor: ts.dtype = np.complex

Normally, you don't want to do that. To be safe, you can do ts = ts.astype(np.complex, copy=False). However, for this case, we just created zeros (and there is a default conversion between 0 float and 0 complex).

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/powellb/seapy/pull/30#issuecomment-253422088, or mute the thread https://github.com/notifications/unsubscribe-auth/AMaBMTpXfLEjkIVXj69X3gY6ylKORxNKks5qzcnNgaJpZM4KVK94.

powellb commented 8 years ago

No, you did it the slow way. I gave the code to use.

dalepartridge commented 8 years ago

Wasn’t sure whether you wanted it that way or the safe way, hence I went for the safe option. Updated again to use ts.dtype=np.complex

On Oct 13, 2016, at 8:26 AM, Brian Powell notifications@github.com wrote:

No, you did it the slow way. I gave the code to use.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/powellb/seapy/pull/30#issuecomment-253597228, or mute the thread https://github.com/notifications/unsubscribe-auth/AMaBMSMaM6ScQCtL8vHU4unhPQ1AJ8Vzks5qznfqgaJpZM4KVK94.

powellb commented 8 years ago

Looks good, accepted.