Skip to content

Conversation

@jkrick
Copy link
Contributor

@jkrick jkrick commented Jan 13, 2026

Mostly just updated to use the IRSA preferred query_ssa() to look for spectra.

For notebook 3, I switched from searching for a spectrum by objectID to searching by coords. I feel this is a more generically useful approach.

Minor other changes:

  • updated date
  • updated author list
  • cleaned up goals for 3rd notebook to reflect notebook (I think they were copied from a different notebook and never checked)

This will close #141

@jkrick jkrick requested a review from bsipocz January 13, 2026 20:59
@jkrick jkrick self-assigned this Jan 13, 2026
@jkrick jkrick added the content: euclid Content related issues/PRs for notebooks with Euclid relevance label Jan 13, 2026
@jkrick
Copy link
Contributor Author

jkrick commented Jan 13, 2026

@tmesh for some reason I can't tag you to review these changes, but wanted to get your thoughts, and eventually seal of approval since you wrote these tutorials.

Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

I have a couple of comments, and one more outstanding todo for myself to see if we can avoid adding the warning handlings.

@@ -1,15 +1,14 @@
---
short_title: "SIR 1D Spectra"
Copy link
Member

Choose a reason for hiding this comment

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

Keep this one, please; if jupyerlab removes it automatically then we need to report it upstream; but in the meantime probably need to do patch git add

```{code-cell} ipython3
# Uncomment the next line to install dependencies if needed
# !pip install matplotlib astropy 'astroquery>=0.4.10'
!pip install matplotlib astropy 'astroquery>=0.4.10'
Copy link
Member

Choose a reason for hiding this comment

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

Keep the comment

Suggested change
!pip install matplotlib astropy 'astroquery>=0.4.10'
# !pip install matplotlib astropy 'astroquery>=0.4.10'

from astroquery.ipac.irsa import Irsa
#suppress warnings about deprecated units
Copy link
Member

@bsipocz bsipocz Jan 13, 2026

Choose a reason for hiding this comment

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

TODO for bsipocz: check if we need this or something has been shipped upstream already; but even if it did it would very likely be in a very recent release so we can't cleanup just yet.

@@ -1,5 +1,4 @@
---
short_title: "SPE Catalogs"
Copy link
Member

Choose a reason for hiding this comment

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

keep the short title

Comment on lines +226 to +229
coord_obj = SkyCoord(
ra=float(obj_row["ra"][0]) * u.deg,
dec=float(obj_row["dec"][0]) * u.deg,
)
Copy link
Member

Choose a reason for hiding this comment

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

Simplify it:

Suggested change
coord_obj = SkyCoord(
ra=float(obj_row["ra"][0]) * u.deg,
dec=float(obj_row["dec"][0]) * u.deg,
)
coord_obj = SkyCoord(obj_row["ra"][0], obj_row["dec"][0], unit=u.deg)

Also, why do we cut this for just the first one?

```

## About this Notebook
###### About this Notebook
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to keep the heading level her

Suggested change
###### About this Notebook
## About this Notebook

Copy link
Member

@bsipocz bsipocz left a comment

Choose a reason for hiding this comment

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

Thank you!

I bunch of smaller comments, the most critical ones is git add changes in batches so we leave the metadata alone as we need that for the rendering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

content: euclid Content related issues/PRs for notebooks with Euclid relevance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: Use SSA for Euclid spectral access notebooks

2 participants