Re: Added new functions to the mapping package

[Sandeep]

Thanks a lot for the suggestions. I am testing the code right now and I will definitely implement all them all as soon as possible.

Sandeep.  

On 5 March 2014 20:44, Felipe G. Nievinski <address@hidden> wrote:

Thanks a lot for your contribution, Sandeep!

A couple -- more like a handful -- of comments:

- tests: did you try to run them, by entering, e.g., "test geodetic2ecef", in the command line?  did it pass, did it fail?  see <https://www.gnu.org/software/octave/doc/interpreter/Test-Functions.html>

- can you make a one centralized script calling all those tests?  I'm not sure if there's a standard filename -- please take a look at the other O-F packages to see how they do this

- this block repeats so often that it deserves to be split as a separate function:

+    if nargin<5 || strcmpi(angleut,'degree')

+    sinfun = @sind;

+    cosfun = @cosd;

+    elseif strcmpi(angleut,'radian')

+    sinfun = @sin;

+    cosfun = @cos;

+    else

+    disperror('Unknown: %s input argument for units',angleut);

+    end

- please always use identation, which is lacking in the block above

- does function disperror exist? I normally use error

- please use error identifiers, not just an error message, when throwing an error

- please investigate whether the "ellipsoid" argument to functions such as ecef2geodetic conforms to whatever matlab uses (I use a structure, it seems they use a vector?);

Would you please address each comment above separately.

Other students, feel free to give Sandeep a hand!  This is supposed to be a collaborative effort.

Thanks,

-Felipe.

On Wed, Mar 5, 2014 at 7:39 AM, Sandeep <address@hidden> wrote:

I have used Felipe's code for Geodetic conversions. For others, I have implemented the standard formulas. Felipe's code gave an error for the ellipsoid name input for string type, so I fixed it so that so that Get_ellipsoid function returned correct values for calculation.

Sincerely,
Sandeep. 

On 5 March 2014 19:56, Juan Pablo Carbajal <address@hidden> wrote:

On Wed, Mar 5, 2014 at 2:54 PM, Sandeep <address@hidden> wrote:
> Hello,
> I have added new functions to the mapping package. Here is my patch:
> https://savannah.gnu.org/patch/index.php?8377
> These are 3d coordinate system conversion functions for Geodesy in Mapping
> package. Please take a look and suggest changes. This is my first time
> contributing to Octave, so please comment if I made some mistakes.
>
> Sincerely,
> Sandeep. (sunnyv)
>
>

I think Felipe G. Nievinski is part of this mailing list, so he can
tell us what have you exactly contributed to the functions.
@Felipe?

--

Sandeep M V 

An Engineer in the making.

--

Sandeep M V 

An Engineer in the making.