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.

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 SkyCoord support to Master #80

Merged
merged 28 commits into from
Apr 22, 2024

Conversation

Xen0Xys
Copy link
Contributor

@Xen0Xys Xen0Xys commented Apr 16, 2024

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

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
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
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
@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
* ✨ 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Take astropy.Skycoords as targets instructions too
3 participants