New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature Request: Accept more formats #2
Comments
For partial angles like 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 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! |
All the above are now supported after c6ec692 I just started a |
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 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 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 |
I just think that 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 |
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:
The text was updated successfully, but these errors were encountered: