search

astropy / astropy-tutorials

Tutorials for the Astropy Project
BSD 3-Clause "New" or "Revised" License
291 stars 176 forks source link

Spectroscopic Data Reduction Basics: Spectroscopic Trace #508

Closed keflavich closed 2 years ago

keflavich commented 2 years ago

This is a resuscitation of #464, #465, but now splitting up into individual notebooks.

There is still some work to do - I need to clean up the frontmatter and remove the output figures before this gets merged.

I'd also like to request review from @tepickering - are there any specreduce tools that should be referenced here? The code used in this example is very low-level, but we could wrap up with, "Here's how you do this more efficiently (in a single command) with specreduce"

review-notebook-app[bot] commented 2 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

tepickering commented 2 years ago

oops! sorry to be so slow to follow up and look at this. we are currently in the process of implementing tracing and extraction within specreduce. the functionality isn't quite ready yet for general use, but it's getting close. once it is, we can decide whether to add "easy mode" sections that use specreduce to existing notebooks or do them in separate notebooks.

in the meantime, look into why the CI checks are failing. it looks like the spaces in the filenames are causing problems...

keflavich commented 2 years ago

I believe I've addressed all of @kelle's comments now

keflavich commented 2 years ago

Bump - can we merge this, or is further review and editing needed?

keflavich commented 2 years ago

Oops, yes, parts II and III already have that prefix. I'll add it.

Otherwise, on specreduce, sounds great - but I think we'd best punt on that edit until there is something legitimately ready. The documentation on the trace routine is pretty near blank?

keflavich commented 2 years ago

@jonathansick I ran nbstripout --extra-keys='metadata.kernelspec metadata.language_info.version metadata.toc' before committing... I don't think the pre-commit should've failed. What command do I need to get this right? https://github.com/astropy/astropy-tutorials/pull/508/commits/c673bceb4a1c2634dfc4a4af5a8c8ba4bffd7ce1

jonathansick commented 2 years ago

@keflavich Here's the full command that's being run in pre-commit:

https://github.com/astropy/astropy-tutorials/blob/b51054da1c1ceec12de5a23a281a0cf93ae3f476/.pre-commit-config.yaml#L18

You might find it easier to run make init to set up pre-commit to run in your local workflow

keflavich commented 2 years ago

Thanks. make init is way too slow for me, I'll just add these to the command