[changeset] behavior of sph2cart() doesn't match matlab (?)

[Ben Abbott]

On Jan 17, 2009, at 3:54 PM, Ben Abbott wrote:

>
> On Jan 17, 2009, at 3:01 PM, John W. Eaton wrote:
>
>> To match the GNU coding standards and most of the rest of Octave
>> please break compound statements before the operator.  Instead of
>>
>> if (! (ismatrix (theta) && ismatrix (r) && ismatrix (z)) ||
>>    ! (size_equal (theta, r) || isscalar (theta) || isscalar (r)) ||
>>    ! (size_equal (theta, z) || isscalar (theta) || isscalar (z)) ||
>>    ! (size_equal (r, z)     || isscalar (r)     || isscalar (z)) )
>>
>> please write
>>
>> if (! (ismatrix (theta) && ismatrix (r) && ismatrix (z))
>>    || ! (size_equal (theta, r) || isscalar (theta) || isscalar (r))
>>    || ! (size_equal (theta, z) || isscalar (theta) || isscalar (z))
>>    || ! (size_equal (r, z) || isscalar (r) || isscalar (z)))
>>
>> In this case, it might be clearer to write
>>
>> if (ismatrix (theta) && ismatrix (r) && ismatrix (z)
>>    && (size_equal (theta, r) || isscalar (theta) || isscalar (r))
>>    && (size_equal (theta, z) || isscalar (theta) || isscalar (z))
>>    && (size_equal (r, z) || isscalar (r) || isscalar (z)))
>>  ... do the operations ...
>> else
>>  error (...)
>> endif
>>
>> because then the statement has fewer negations so might be clearer
>> about what conditions are necessary to do the operations, rather that
>> what conditions are necessary for an error.
>
> I agree, that is clearer.
>
>>
>> Also, most error messages don't end with periods.  If you think that
>> seems wrong, then I think we should discuss it and do something
>> consistent for all messages in Octave.
>
> No problem, I'll stick to the current practice.
>
> Ben

New changeset attached. I noticed that I neglected to account for  
cart2pol and pol2cart accepting 2 arguments. I've fixed that as well  
and included additional tests.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: changeset-cart2sph.patch
Type: application/octet-stream
Size: 11704 bytes
Desc: not available
Url : https://www-old.cae.wisc.edu/pipermail/bug-octave/attachments/20090117/a07b65ef/attachment-0001.obj 
-------------- next part --------------



Ben

p.s. I just noticed we dropped of the list, I assume that was  
unintended?