Adstring outputs wrong string with negative zero

[G-Francio]

Adstring outputs the wrong string when dec starts with negative zero (i.e. -0.5), while radec converts correctly.

>> adstring(15, -0.5)
" 01 00 00.0  +00 30 00"

>> radec(15, -0.5)
(1.0, 0.0, 0.0, -0.0, 30.0, 0.0)

The bug seems to be this line in adstring.jl:

dec_string = @sprintf("%+03.2d %02d %s", dec_deg, dec_min, dec_sec_string)

which ignores the negative zero. A possible fix (although, since I'm a beginner, probably not a reliable one) could be: dec_string = (dec > 0 ? "+" : "-") * @sprintf("%02.2d %02d %s", abs(dec_deg), dec_min, dec_sec_string)

[Aman-Pandey-afk]

I want to work on this issue, how should I get started? I already have build AstroLib and tried to modify according to the suggestion mentioned (though rebuilding the pkg doesn't reflect it).

[giordano]

I don't know what you mean by "I have already build AstroLib", but the best way to work on a Julia package is to develop it. In the Julia REPL, type ] to enter Pkg REPL mode, then run the command

develop AstroLib

or in short

dev AstroLib

This will have created a git clone of this repository under your .julia directory, in particular in .julia/dev/AstroLib. Then, you can edit the source code of the package in this directory and when you restart the Julia session you'll see the changes taking effect (in the same environment as where you developed the package). Note that you can avoid restarting the Julia sessions if you're using Revise.jl.

Once you're done with the changes, you can submit a pull request here. Note that it's always good practice to add new tests in a pull request which is fixing a bug. Let me know if you need more guidance about how to open a pull request.

[Aman-Pandey-afk]

I looked into the problem and found that the above suggestion by G-Francio requires just int casting to produce the correct output. The first one is his output and second one is mine after modifying abs(dec_deg) to Int(abs(dec_deg)). Condition dec>0 also needed modification to dec>=0

julia> adstring(25, -0.6)
" 01 40 00.0  - 0 36 00"

julia> adstring(25, -0.6)
" 01 40 00.0  -00 36 00"

The modified line: dec_string = (dec >= 0 ? "+" : "-") * @sprintf("%02.2d %02d %s", Int(abs(dec_deg)), Int(dec_min), dec_sec_string)

I have tried it for several tests, it is good looking for every case. Also, we can use just dec_string = @sprintf("%+02.2d %02d %s", dec_deg, Int(dec_min), dec_sec_string) This outputs like following (which maybe undesirable) :

adstring(25, -0.6)
" 01 40 00.0  -0 36 00"

It maybe a help to know if there is any guidelines for pull requests, or just I create a new branch (Pushing the AstroLib directory inside .julia/dev) and make a pr like usual.

[Aman-Pandey-afk]

@giordano it would be great of you if you could provide some feedback on this and also contribution practices you follow. I would be glad to work on any other issue also, if you could point it out.

[giordano]

It maybe a help to know if there is any guidelines for pull requests, or just I create a new branch (Pushing the AstroLib directory inside .julia/dev) and make a pr like usual.

Yes, that's absolutely fine. We can discuss the details of the implementation in the pull request