Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Merge SkyCoord support to Master #80

Merged
merged 28 commits into from
Apr 22, 2024
Merged

Merge SkyCoord support to Master #80

merged 28 commits into from
Apr 22, 2024

Conversation

Xen0Xys
Copy link
Contributor

@Xen0Xys Xen0Xys commented Apr 16, 2024 •
edited
Loading

TODO

  • Fix positionChanged event when position.ra/dec is different that getRaDec result
  • Fix example 6 bug with position sync between multiple aladin views
  • Add SkyCoord when the target is a coordinate string
  • Add documentation
  • Check that existing examples are always working
  • Fix target initialization parameter
  • Add testing for parsing methods
  • Add changelog

@ManonMarchand
Copy link
Member

ManonMarchand commented Apr 16, 2024 •
edited
Loading

Thanks Tom!

Fixes: #51

For the gitignore, you can edit your local .git/info/exclude to avoid editing the shared one

I'll have a look at the rest tomorrow 🙂

Edit: could you rebase on master? I tried to fix the codestyle workflow

@ManonMarchand ManonMarchand linked an issue Apr 16, 2024 that may be closed by this pull request
Take astropy.Skycoords as targets instructions too #51
Closed
bmatthieu3
bmatthieu3 requested changes Apr 17, 2024
View reviewed changes
Copy link
Collaborator

@bmatthieu3 bmatthieu3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Xen0Xys . I do have some little things to suggest. Overall it is very good :D

src/ipyaladin/__init__.py Outdated Show resolved Hide resolved
src/ipyaladin/__init__.py Outdated Show resolved Hide resolved
src/ipyaladin/__init__.py Outdated Show resolved Hide resolved
src/ipyaladin/__init__.py Outdated Show resolved Hide resolved
@Xen0Xys Xen0Xys force-pushed the feature/skycoord-support branch from 1007818 to 3490795 Compare April 17, 2024 08:01
@Xen0Xys Xen0Xys force-pushed the feature/skycoord-support branch from 1af15df to c229db9 Compare April 17, 2024 12:42
@Xen0Xys Xen0Xys force-pushed the feature/skycoord-support branch from 39f98f6 to 4b8e898 Compare April 18, 2024 09:01
@Xen0Xys Xen0Xys force-pushed the feature/skycoord-support branch from e531c91 to 9c36e6b Compare April 18, 2024 10:13
@Xen0Xys Xen0Xys force-pushed the feature/skycoord-support branch from 4b4a753 to e9ae30b Compare April 18, 2024 10:36
ManonMarchand
ManonMarchand requested changes Apr 18, 2024
View reviewed changes
js/widget.js Show resolved Hide resolved
src/ipyaladin/__init__.py Show resolved Hide resolved
@Xen0Xys Xen0Xys marked this pull request as ready for review April 18, 2024 11:59
@Xen0Xys Xen0Xys requested a review from ManonMarchand April 18, 2024 11:59
@Xen0Xys Xen0Xys requested a review from ManonMarchand April 22, 2024 09:48
@ManonMarchand ManonMarchand merged commit c694c1a into cds-astro:master Apr 22, 2024
2 checks passed
@ManonMarchand
Copy link
Member

Merged, thanks Tom! 🎉

ManonMarchand pushed a commit that referenced this pull request Apr 22, 2024
@Xen0Xys @ManonMarchand
feat: add support of SkyCoord for the target property (#80)
981833f 
* ✨ Implement SkyCoord support for js side

* ✨ Implement SkyCoord support for python side

* 🐛 Fix target support for jslink using shared_target traitlets

* 📝 Update example 6 to fix target jslink behavior

* 🐛 Fix multi-instances flickering

* 🐛 Fix useless target when used in constructor

* ✨ Implement coordinate string parsing using new coordinate_parser function

* 📝 Add minor documentation for widget.js

* 🚀 Add missing target getter type annotation

* 🐛 Fix coordinate_parser.py splitter

* ✅ Add tests for coordinate_parser.py and aladin target getter and setter

* 👷 Add CI for automated python code testing

* ✅ Add more testing cases for existing tests

* 🐛 Fix initial target when frame is galactic

* 🎨 Use a regex to detect if a string is an object name or a coordinate

* 📝 Update CHANGELOG.md

* 📝 Add _target and shared_target trait help & remove useless error in trait getter

* 🎨 Fix python import order

* 🐛 Fix missing event unsubscribe for shared_target

* 🎨 Improve conditional structure for the target setter

* 📝 Improve the meaning of a sentence in the changelog

* 🎨 Improve target setter conditions

* ✨ Add parsing support for coordinate string starting by J, G and B

* ✅ Improve testing for new coordinate parsing functions

* 📝 Change docstring format from sphinx to numpy

* 🎨 Improve conditional structure for parse_coordinate_string function

* fix: type annotations union for python <3.9

* docs: fix doctring style
@Xen0Xys Xen0Xys deleted the feature/skycoord-support branch April 23, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmatthieu3 bmatthieu3 Awaiting requested review from bmatthieu3

@ManonMarchand ManonMarchand Awaiting requested review from ManonMarchand

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Take astropy.Skycoords as targets instructions too
3 participants
@Xen0Xys @ManonMarchand @bmatthieu3