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

Adding SDSS APOGEE notebook tutorial #93

Merged
merged 4 commits into from
Jan 15, 2025

Conversation

astrojimig
Copy link

Adding the notebook tutorial for SDSS APOGEE data, which is now available in ops!

Tagging @ttdu @havok2063 @snbianco for review/visibility

@astrojimig astrojimig requested a review from ttdu as a code owner January 6, 2025 16:17
@astrojimig
Copy link
Author

astrojimig commented Jan 6, 2025

@snbianco This notebook requires the URL-escaping updates you made to astroquery.mast a few months ago (astropy/astroquery#3080) Do you happen to know when that will be part of a tagged release?

My local environment is using 0.4.8.dev9672, which works, but the pipeline here lists 0.4.8.dev9474 as the latest version, which does not work (see output).

I'm not sure how best to proceed - do we need to wait until the astroquery updates are in a tagged release to merge this notebook in?

Edit: Building from source in the requirements.txt seems to work! We should update this with a tagged release when we can, but for now I think this is okay!

@snbianco
Copy link
Contributor

snbianco commented Jan 6, 2025

Yes, I think that a new tagged release of Astroquery is coming out in the near future! Until then, installing from the GitHub repo should work.

updating TOC

fixing linting for APOGEE notebook

more small edits

more small linting edits

updating APOGEE requirements for astroquery url encoding

updating requirements for APOGEE
@astrojimig
Copy link
Author

Squashed some commits now that the pipeline is passing, this should be ready to go now!

@havok2063
Copy link

This looks good to me.

@astrojimig
Copy link
Author

@snbianco @ttdu Any chance we can merge this sometime today?

@snbianco
Copy link
Contributor

I think this is an awesome tutorial notebook with a lot of great information! The plots look really cool, and I like the comparison of 3 different variable stars at the end. I had some comments, but mostly about small things in the notebook text:

  • In “Searching for a specific star”, the text says that we’ll use the obs_id keyword, but the code uses target_name.
  • Typo on the word “available” in “Downloading APOGEE data products”.
  • Maybe add a note in “Downloading the APOGEE allStar Catalog” that the file download will take a while.
  • Typo on the word “handfull” in “Coordinate search using astroquery.mast”.
  • Finding the closest match via distance makes sense, but all of the results from the query have the same coordinates and distance. Maybe just make a note on that particular example that all of the observations have the same distance, so it will choose the first row.

@astrojimig
Copy link
Author

Thank you @snbianco ! I just pushed an update addressing your comments.

@snbianco snbianco merged commit 557aaf2 into spacetelescope:main Jan 15, 2025
8 checks passed
@astrojimig
Copy link
Author

Thank you @snbianco !

@adrianlucy
Copy link
Contributor

The html render looks good to me at a glance, but let us know if you notice any issues @astrojimig! https://spacetelescope.github.io/mast_notebooks/notebooks/SDSS/APOGEE_TESS_tutorial/APOGEE_TESS_tutorial.html

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.

4 participants