-
Notifications
You must be signed in to change notification settings - Fork 27
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
Merge Astropy Angle support to Master #83
Merged
bmatthieu3
merged 11 commits into
cds-astro:master
from
Xen0Xys:feature/fov-angle-support
Apr 29, 2024
Merged
Changes from 9 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
c057f97
:sparkles: Start implementing fov as an astropy.Angle
Xen0Xys c412510
:arrow_up: Update Aladin Lite version from 3.3.3 to 3.4.0
Xen0Xys 580c372
:memo: Update example 6 to replace fov by shared_fov
Xen0Xys 7480bd9
:memo: Add documentation for _fov and shared_fov traits
Xen0Xys 08e8b7b
:fire: Remove unnecessary Aladin#setFoV method call
Xen0Xys 69be362
:memo: Fix example 10
Xen0Xys d5d6924
:memo: Fix example 7
Xen0Xys d73d8ec
:memo: Update changelog and fix typo
Xen0Xys 42c93ce
:white_check_mark: Add integration testing cases for Angle fov
Xen0Xys 8433cc9
:memo: Improve documentation clarity
Xen0Xys 6c53198
:memo: Update aladin correspondence table
Xen0Xys File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import pytest | ||
from astropy.coordinates import Angle | ||
|
||
from ipyaladin import Aladin, parse_coordinate_string | ||
|
||
test_aladin_string_target = [ | ||
"M 31", | ||
"sgr a*", | ||
"α Centauri", # noqa RUF001 | ||
"* 17 Com", | ||
"1:12:43.2 31:12:43", | ||
"1:12:43.2 +31:12:43", | ||
"1:12:43.2 -31:12:43", | ||
"1 12 43.2 31 12 43", | ||
"1 12 43.2 +31 12 43", | ||
"1 12 43.2 -31 12 43", | ||
"1h12m43.2s 1d12m43s", | ||
"1h12m43.2s +1d12m43s", | ||
"1h12m43.2s -1d12m43s", | ||
"42.67 25.48", | ||
"42.67 +25.48", | ||
"42.67 -25.48", | ||
"0 0", | ||
"J42.67 25.48", | ||
"G42.67 25.48", | ||
"B42.67 25.48", | ||
"J12 30 45 +45 30 15", | ||
"J03 15 20 -10 20 30", | ||
"G120.5 -45.7", | ||
"G90 0", | ||
"B60 30", | ||
"B120 -45", | ||
] | ||
|
||
|
||
@pytest.mark.parametrize("target", test_aladin_string_target) | ||
def test_aladin_string_target_set(target): | ||
"""Test setting the target of an Aladin object with a string or a SkyCoord object. | ||
|
||
Parameters | ||
---------- | ||
target : str | ||
The target string. | ||
|
||
""" | ||
aladin = Aladin() | ||
aladin.target = target | ||
parsed_target = parse_coordinate_string(target) | ||
assert aladin.target.icrs.ra.deg == parsed_target.icrs.ra.deg | ||
assert aladin.target.icrs.dec.deg == parsed_target.icrs.dec.deg | ||
|
||
|
||
@pytest.mark.parametrize("target", test_aladin_string_target) | ||
def test_aladin_sky_coord_target_set(target): | ||
"""Test setting and getting the target of an Aladin object with a SkyCoord object. | ||
|
||
Parameters | ||
---------- | ||
target : str | ||
The target string. | ||
|
||
""" | ||
sc_target = parse_coordinate_string(target) | ||
aladin = Aladin() | ||
aladin.target = sc_target | ||
assert aladin.target.icrs.ra.deg == sc_target.icrs.ra.deg | ||
assert aladin.target.icrs.dec.deg == sc_target.icrs.dec.deg | ||
|
||
|
||
test_aladin_float_fov = [ | ||
0, | ||
360, | ||
180, | ||
-180, | ||
720, | ||
] | ||
|
||
|
||
@pytest.mark.parametrize("angle", test_aladin_float_fov) | ||
def test_aladin_float_fov_set(angle): | ||
"""Test setting the angle of an Aladin object with a float. | ||
|
||
Parameters | ||
---------- | ||
angle : float | ||
The angle to set. | ||
|
||
""" | ||
aladin = Aladin() | ||
aladin.fov = angle | ||
assert aladin.fov.deg == angle | ||
|
||
|
||
@pytest.mark.parametrize("angle", test_aladin_float_fov) | ||
def test_aladin_angle_fov_set(angle): | ||
"""Test setting the angle of an Aladin object with an Angle object. | ||
|
||
Parameters | ||
---------- | ||
angle : float | ||
The angle to set. | ||
|
||
""" | ||
angle_fov = Angle(angle, unit="deg") | ||
aladin = Aladin() | ||
aladin.fov = angle_fov | ||
assert aladin.fov.deg == angle_fov.deg |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to remove the goToRaDec after initialization with this bump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this should be in the changelog and in the readme
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite the version change, the removing of gotoRaDec call at the initialization re-introduce the patched bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too bad, thanks for testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the correspondence table at the end of the README, could you add a line with
unreleased || 3.4.0-beta
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is because when you instanciate aladin with:
as there is no frame given with the target position string, it will take that position as what is given in the cooFrame.
@tboch - Should we consider that behaviour as okaish or
target
should be always considered as given in ICRS frame ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done !