vtsuperdarn / davitpy

DEPRECATED The DaViT Python project
http://vtsuperdarn.github.com/davitpy/
GNU General Public License v3.0
37 stars 59 forks source link

Updated __init__.py for assert issues with opt args #331

Closed RichardoC closed 6 years ago

RichardoC commented 6 years ago

Currently this will always fail as the Truth value of an array is ambiguous so I've rewritten it to have the expected behavior.

asreimer commented 6 years ago

The bug you are fixing is due to changed behaviour in numpy after version 1.12.0. There was ambiguity in how conditional logic with arrays was done, so they made it less ambiguous.

A temporary fix is to use numpy==1.12.0 instead. Long term, conditional logic using arrays needs to be changed, as you've done here for this it of code.

Can you please make this pull request into the develop branch? We do not merge things directly in to the master branch.

RichardoC commented 6 years ago

@asreimer , done though I don't think it's a numpy issue. The code I changed is using python lists, not numpy arrays.

aburrell commented 6 years ago

@RichardoC , since we don't have unit tests yet, do you have a bit of code you recommend using as a test piece?

asreimer commented 6 years ago

@RichardoC we need code that you used to produce this error. If we can't reproduce the error we can't tell if the changes you made are fixing it.

We will close this in a week if we don't hear back.

edit: Closing now. Pull request doesn't fix issue.

asreimer commented 6 years ago

Ok, I can reproduce the error that @RichardoC, but only using numpy arrays. Try this code:

from numpy import arange, zeros, ones
from davitpy.models import tsyganenko

lats = arange(10, 90, 10)
lons = zeros(len(lats))
rhos = 6372.*ones(len(lats))
trace = tsyganenko.tsygTrace(lats, lons, rhos)

and you get:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-7-5db7be2384f8> in <module>()
----> 1 trace = tsyganenko.tsygTrace(lats, lons, rhos)

/home/ashtonsethreimer/davitpy/davitpy/models/tsyganenko/__init__.py in __init__(self, lat, lon, rho, filename, coords, datetime, vswgse, pdyn, dst, byimf, bzimf, lmax, rmax, rmin, dsmax, err)
    129         from datetime import datetime as pydt
    130 
--> 131         assert (None not in [lat, lon, rho]) or filename, 'You must provide either (lat, lon, rho) or a filename to read from'
    132 
    133         if None not in [lat, lon, rho]:

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()

If you use lists, like @RichardoC claims, you run code like this:

trace = tsyganenko.tsygTrace(lats.tolist(), lons.tolist(), rhos.tolist())

and get:

    ---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-10-b21acc359507> in <module>()
----> 1 trace = tsyganenko.tsygTrace(lats.tolist(), lons.tolist(), rhos.tolist())

/home/ashtonsethreimer/davitpy/davitpy/models/tsyganenko/__init__.py in __init__(self, lat, lon, rho, filename, coords, datetime, vswgse, pdyn, dst, byimf, bzimf, lmax, rmax, rmin, dsmax, err)
    148             if not iTest: self.__del__()
    149 
--> 150             self.trace()
    151 
    152         elif filename:

/home/ashtonsethreimer/davitpy/davitpy/models/tsyganenko/__init__.py in trace(self, lat, lon, rho, coords, datetime, vswgse, pdyn, dst, byimf, bzimf, lmax, rmax, rmin, dsmax, err)
    353                     self.zTrace[ip,0:l] = zarr[l-1::-1]
    354                 elif mapto == 1:
--> 355                     self.xTrace[ip,self.l[ip]:self.l[ip]+l] = xarr[0:l]
    356                     self.yTrace[ip,self.l[ip]:self.l[ip]+l] = yarr[0:l]
    357                     self.zTrace[ip,self.l[ip]:self.l[ip]+l] = zarr[0:l]

TypeError: slice indices must be integers or None or have an __index__ method

As I originally stated, this bug is fundamentally caused by a numpy change after version 1.12.0.

So with that out of the way, I tested this pull request using both:

trace = tsyganenko.tsygTrace(lats, lons, rhos)
trace = tsyganenko.tsygTrace(lats.tolist(), lons.tolist(), rhos.tolist())

The current pull request does not fix the issue.

What does fix the issue is:

1) replace line 131 with: assert not (lat is None) and not (lon is None) and not (rho is None) or not (filename is None), 'You must provide either (lat, lon, rho) or a filename to read from' 2) replace line 133 with: if filename is None: 3) replace line 152 with else:

So I'm going close this pull request and open a new one that fixes the issue.