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

feat: layers manipulations #84

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

Xen0Xys
Copy link
Contributor

@Xen0Xys Xen0Xys commented Apr 25, 2024

Fixes #76

@bmatthieu3
Copy link
Collaborator

This is just an idea I have that would allow the user to compose its HiPS stack fast with several HiPSes with colormap etc...

It would be nice to have a new traitlet targeting the global aladin image stack (HiPS and FITS) stack. I see it as a python list of object such as:

aladin.stack = [{
    "url": "https://alasky.cds.unistra.fr/SDSS/DR9/band-g/", # HTTPS url pointing towards the HiPS
    "opacity": 0.2,
    "colormap": "rainbow",
    "stretch": "asinh", # apply stretch function to the pixel before the colormap
    "format": "fits", # format of the tiles
    "minCut": 100,
    "maxCut": 2000
  },
  {
   "id": "CDS/P/2MASS/color",
    "opacity": 0.4,
    "format": "png",
  }
]

This list describes a view with 2 HiPS, 2MASS on top of SDSS9/g. For one HiPS, one should only accept "id" or "url" for identifying an HiPS. If both are given, we would take ID and return a warning that the url has not been taken into account for retrieving the HiPS. Eventually, we could only accept id and the user could pass an HTTPS url to the id keyword too. This is better I think.

Something should be done to sync the overlay_survey and overlay_opacity traitlets with this one (i.e. aladin.stack[1].url/id and aladin.stack[1].opacity)

@bmatthieu3
Copy link
Collaborator

You could even put some astropy image HDU in that stack list:

aladin.stack = [
  "CDS/P/2MASS/color", # or only the ID
  "https://mastimg.stsci.edu/surveys/hips/DSS/DSSColor/", # only an url pointing to the root of an HiPS
  {
    "url": "https://alasky.cds.unistra.fr/SDSS/DR9/band-g/", # HTTPS url pointing towards the HiPS
    "opacity": 0.2,
    "colormap": "rainbow",
    "stretch": "asinh", # apply stretch function to the pixel before the colormap
    "format": "fits", # format of the tiles
    "minCut": 100,
    "maxCut": 2000
  }, # A HiPS with more optional parameters
  astropy.io.fits.ImageHDU # cf https://docs.astropy.org/en/stable/io/fits/api/images.html#astropy.io.fits.ImageHDU
]

The above list describes a stack of 4 layers:

  1. The base is 2MASS/color
  2. On top the DSS hosted by MAST
  3. On top is a HiPS of SDSS9/g with opacity 20% and a specific colormap
  4. On top of all the HiPS, a FITS Image HDU astropy object.

@Xen0Xys Xen0Xys closed this May 7, 2024
@Xen0Xys Xen0Xys force-pushed the feature/add-hips-support branch from 6652312 to 39415a9 Compare May 7, 2024 07:42
@Xen0Xys Xen0Xys reopened this May 21, 2024
@Xen0Xys Xen0Xys marked this pull request as ready for review May 21, 2024 08:04
@Xen0Xys Xen0Xys requested a review from ManonMarchand May 22, 2024 11:19
@ManonMarchand
Copy link
Member

@bmatthieu3 : ready to merge?

@bmatthieu3
Copy link
Collaborator

@ManonMarchand @Xen0Xys - I think we also need to consider sync the overlay_survey and overlay_survey_opacity traitlet. If you add a hips survey with the add_hips method we should update the value of overlay_survey, i.e. a call to add_hips should be sync with overlay_survey (the first overlay should match overlay_survey).

aladin.add_hips('https://alasky.cds.unistra.fr/DES/DR2/CDS_P_DES-DR2_ColorIRG/')
aladin.add_hips('SDSS9/color')
aladin.overlay_survey # should return the id/url from the last overlay added i.e. 'SDSS9/color'

@Xen0Xys
Copy link
Contributor Author

Xen0Xys commented May 23, 2024

@ManonMarchand @Xen0Xys - I think we also need to consider sync the overlay_survey and overlay_survey_opacity traitlet. If you add a hips survey with the add_hips method we should update the value of overlay_survey, i.e. a call to add_hips should be sync with overlay_survey (the first overlay should match overlay_survey).

aladin.add_hips('https://alasky.cds.unistra.fr/DES/DR2/CDS_P_DES-DR2_ColorIRG/')
aladin.add_hips('SDSS9/color')
aladin.overlay_survey # should return the id/url from the last overlay added i.e. 'SDSS9/color'

Currently, the HiPS is added as a layer and not an overlay:
this.aladin.addNewImageLayer(hips);
It seems counterintuitive to sync the overlay with a non-overlay layer, doesn't it?

@ManonMarchand
Copy link
Member

ManonMarchand commented May 23, 2024

I also find it strange. If we have two methods then why not leave the users the freedom to manipulate the overlay separately?

@bmatthieu3
Copy link
Collaborator

bmatthieu3 commented May 23, 2024

addNewImageLayer adds a new overlay survey so I do not get your points. An overlay has a name, this name is called layer, that is maybe why it is confusing. Internally, addNewImageLayer calls setOverlayImageLayer with your survey and a arbitrary layer name.

@Xen0Xys
Copy link
Contributor Author

Xen0Xys commented May 23, 2024

The overlay behavior change between addNewImageLayer and setOverlayImageLayer.
With the first one, the second call to add_hips add a third overlay on top on the second.
With the second one, the second call to add_hips replace the previous added hips.

Which behavior do you want for the implementation?
In addition, how to manage the overlay_survey_opacity traitlets? Cause if we use the first behavior, I don't know if getOverlayImageLayer return something.

@bmatthieu3
Copy link
Collaborator

@ManonMarchand @tboch - I guess my point is: what do we want to do with the 'overlay_survey' traitlet since it is now possible to add multiple hips surveys with this method. For example, if a user adds 3 hips by calling 3 times add_hips and then change the overlay_survey traitlet what should it do ? Different possible cases:

  • overlay_survey changes the very top layer (the 3rd added hips)
  • overlay_survey changes the first added layer, so we may not see the change on screen because there are 2 more on top.
  • overlay_survey adds a 4th hips
    What do you think ? I tend to prefer the first case

I also ask myself if it would not be better to have a 'set_hips(hips, name)' instead of add_hips so that the user controls better the stack i.e. it is possible to change any layer like that by specifying its name. If no name is given, the first base layer is taken.

@tboch
Copy link
Contributor

tboch commented May 24, 2024

@bmatthieu3 I think overlay_survey should reflect the reality, and become an array, thus overlay_surveys

@Xen0Xys
Copy link
Contributor Author

Xen0Xys commented May 24, 2024

@bmatthieu3 I think overlay_survey should reflect the reality, and become an array, thus overlay_surveys

So, we need an event when the survey list is updated on the AL UI, because otherwise, we can't track the overlay list changes corrrectly.

@ManonMarchand ManonMarchand marked this pull request as draft July 15, 2024 12:36
@Xen0Xys Xen0Xys force-pushed the feature/add-hips-support branch from ee6cc2e to cb34db4 Compare August 2, 2024 14:50
src/ipyaladin/widget.py Outdated Show resolved Hide resolved
src/ipyaladin/widget.py Outdated Show resolved Hide resolved
src/ipyaladin/widget.py Outdated Show resolved Hide resolved
src/ipyaladin/widget.py Outdated Show resolved Hide resolved
js/models/event_handler.js Show resolved Hide resolved
@ManonMarchand ManonMarchand force-pushed the feature/add-hips-support branch from 2e76964 to e6f173f Compare September 18, 2024 13:42
@ManonMarchand ManonMarchand changed the base branch from master to dev September 27, 2024 09:50
@ManonMarchand ManonMarchand changed the title Merge add hips to Master feat: layers manipulations Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants