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

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

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.

@jkrick
Copy link
Contributor Author

jkrick commented Jan 14, 2026

@bsipocz I will try my best to do the adding in patches for future PRs, I know you have mentioned this before, its just not yet part of my muscle memory. For this PR I have manually added back in the shortitles since that seemed easier than messing with the commit history. I believe all the other changes are done.

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.

All looks good to me, but I don't merge it myself in case you want to wait for Tiffany's feedback.

@jkrick
Copy link
Contributor Author

jkrick commented Jan 20, 2026

Im going to merge this as it has become relevant to a different slack discussion. @tmesh if you have any comments, please open an issue for them.

@jkrick jkrick merged commit a54578e into Caltech-IPAC:main Jan 20, 2026
8 checks passed
@jkrick jkrick deleted the use_query_ssa branch January 20, 2026 17:54
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