Feature Request: Accept more formats
briochemc opened this issue · 9 comments
I'd like to suggest expanding the accepted coordinate formats. The goal would be able to parse these:
-
1°1′1
(minute symbol made with\prime + TAB
) -
1°1'
(no seconds) -
1°
(no minutes) -
1°1'1W
(North/South/West/East direction at the end)
Here is a little snippet to test these:
julia> strings_to_test = Dict(
"expected" => "1°1'1",
"\\prime" => "1°1′1",
"no seconds" => "1°1'",
"no minutes" => "1°",
"NSEWdirection" => "1°1'1W"
) ;
julia> function myparse(str)
try
parse_dms(str)
catch e
"does not parse"
end
end
myparse (generic function with 1 method)
julia> vcat(([k v myparse(v)] for (k,v) in strings_to_test)...)
5×3 Matrix{Any}:
"expected" "1°1'1" (1.0, 1.0, 1.0)
"no minutes" "1°" "does not parse"
"NSEWdirection" "1°1′1W" "does not parse"
"no seconds" "1°1'" "does not parse"
"\\prime" "1°1'1" "does not parse"
For partial angles like "1°1'"
- are there real use-cases where coordinates are saved in a partial format? For REPL usage, the extra few characters to write ``"1°1'0"` seems like a much simpler solution than building in another two layers of logical complexity to the regex strings.
As for the other formats, I'm not too eager to build in every corner-case under the sun, but I see astropy supports a few of them so I guess feature parity wins out there.
I see astropy supports the partial angles, too, so I'll add them into the PR I'm working on.
Yes, here is one example: https://www.sciencedirect.com/science/article/pii/S0009254119300075?via%3Dihub#t0015 in the wild. (I "manually" did the conversion from X°Y'W
to -X-Y/60
in this case.)
I strongly agree about the REPL use case. But we shouldn't ignore the non-REPL use cases, right?
I think the more formats are supported, the better, but I don't want to sound like an entitled user! Feel free to chose and select what you think is worth implementing / including in the package!
This looks great! However, I think there is a bug or two remaining. I noticed that North/South is treated differently from East/West (one is for degrees, the other for hours). Could you explain why this is the case?
I did not tick the 4th example in the list at the start of this thread, namely 1°1'1W
, because AstroAngles (v0.1.2) currently returns a positive value instead of a negative one.
Here is another MWE that could serve as a testbed (displayed are: original string, parsed output, expected output, and a boolean to test equality between the current and expected output):
julia> strings_to_test = ["1$sep$d" for sep in ["°", "'", "''"] for d in ["N", "S", "E", "W"]] ;
julia> expected_output = [(occursin(d,"NE") ? 1.0 : -1.0) .* (sep=="°" ? (1,0,0) : (sep=="'" ? (0,1,0) : (0,0,1))) for sep in ["°", "'", "''"] for d in ["N", "S", "E", "W"]] ;
julia> function myparse(str)
try
parse_dms(str)
catch e
"does not parse"
end
end
myparse (generic function with 1 method)
julia> vcat(([v myparse(v) e myparse(v)==e] for (v,e) in zip(strings_to_test,expected_output))...)
12×4 Matrix{Any}:
"1°N" (1.0, 0.0, 0.0) (1.0, 0.0, 0.0) true
"1°S" (-1.0, 0.0, 0.0) (-1.0, -0.0, -0.0) true
"1°E" (1.0, 0.0, 0.0) (1.0, 0.0, 0.0) true
"1°W" (1.0, 0.0, 0.0) (-1.0, -0.0, -0.0) false
"1'N" (1.0, 0.0, 0.0) (0.0, 1.0, 0.0) false
"1'S" (-1.0, 0.0, 0.0) (-0.0, -1.0, -0.0) false
"1'E" (1.0, 0.0, 0.0) (0.0, 1.0, 0.0) false
"1'W" (1.0, 0.0, 0.0) (-0.0, -1.0, -0.0) false
"1''N" (1.0, 0.0, 0.0) (0.0, 0.0, 1.0) false
"1''S" (1.0, 0.0, 0.0) (-0.0, -0.0, -1.0) false
"1''E" (1.0, 0.0, 0.0) (0.0, 0.0, 1.0) false
"1''W" (1.0, 0.0, 0.0) (-0.0, -0.0, -1.0) false
As you can see the West direction is not taken into account (it's only implemented for hms
) and in the absence of degrees the parsing silently assumes that minutes and seconds are degrees...
I'm also happy to try and submit a PR if the expected output above is what you'd want.
I see the problem- I just don't understand it. Do you have a reference or can write out exactly how the directions should change the angle? I started with N/S and E/W because that's what made sense for e.g. ICRS coordinates. Similarly, the sign convention for hms
is based off the hour angle parsing that astropy does- so again if you have some reference that would be the best clarification.
I just think that "1°W"
should be parsed like "-1°E"
(i.e., as a negative).
Although I'm not familiar with that package, looking at astropy's tests seems to confirm this (but maybe I'm confused?):
a = Angle("3dN")
assert str(a) == "3d00m00s"
assert a.unit == u.degree
a = Angle("3dS")
assert str(a) == "-3d00m00s"
assert a.unit == u.degree
a = Angle("3dE")
assert str(a) == "3d00m00s"
assert a.unit == u.degree
a = Angle("3dW")
assert str(a) == "-3d00m00s"
assert a.unit == u.degree
a = Angle('10"')
assert_allclose(a.value, 10.0)
assert a.unit == u.arcsecond
a = Angle("10'N")
assert_allclose(a.value, 10.0)
assert a.unit == u.arcminute
a = Angle("10'S")
assert_allclose(a.value, -10.0)
assert a.unit == u.arcminute
a = Angle("10'E")
assert_allclose(a.value, 10.0)
assert a.unit == u.arcminute
a = Angle("10'W")
assert_allclose(a.value, -10.0)
assert a.unit == u.arcminute
a = Angle('45°55′12″N')
assert str(a) == '45d55m12s'
assert_allclose(a.value, 45.92)
assert a.unit == u.deg
a = Angle('45°55′12″S')
assert str(a) == '-45d55m12s'
assert_allclose(a.value, -45.92)
assert a.unit == u.deg
a = Angle('45°55′12″E')
assert str(a) == '45d55m12s'
assert_allclose(a.value, 45.92)
assert a.unit == u.deg
a = Angle('45°55′12″W')
assert str(a) == '-45d55m12s'
assert_allclose(a.value, -45.92)
assert a.unit == u.deg
Okay, I've added the cardinal directions for both hms and dms. The convention will be "W" and "S" are negative.
I took a stab at implementing when only one digit is provided but isn't degrees, I didn't find a great solution.
I started a branch myself by splitting the huge regex in smaller chunks and I think that I stumbled onto an error.
What should "-1:2:3"
parse to? IMHO the minus sign should propagate all the way, but currently the sign only matters for the degrees/hours. But maybe I'm wrong?