Closed joelburke02 closed 2 years ago
Thank you @joelburke02 for your Pull Request to muler
! 🥳 It is community contributions like yours that keep muler
going. 🌅
I see that your notebook reads in genuine astrophysical HPF spectra with muler
and and shows a technique to combine them with error propagation. The codebase does not currently include a tutorial illustrating this approach, so yours will be a welcomed addition.
I see a few ways to augment your notebook to make it accessible to a wider audience:
Location of the file
I see that the notebook has been placed in the project's root directory muler/Combining Spectra Example.ipynb
. Please move it to the same place as the other tutorials: muler/docs/tutorials/Combining Spectra Example.ipynb
. While you are at it, can you replace the spaces with underscores? Different Operating Systems handle spaces differently and can sometimes cause friction with version control.
Enable the notebook to appear as a standalone tutorial on the documentation website
Within the muler/docs/tutorials/
directory you will see an index.rst
file. Add a line to that file mimicking the format for the other tutorials, e.g.: HPF Sky Subtraction and Deblazing <HPF_tutorial.ipynb>
but adjusted for the title and name of your notebook.
Use of imports
I recommend against the pattern from pylab import *
. It appears you only need one method from pylab, plot
, which could be replaced with your existing pattern of plt.plot()
.
Contiguous notebook cell numbers
Before re-uploading the notebook, select "Restart and Run All Cells" from the notebook's Kernel
dropdown menu.
Don't repeat yourself
Cells 21-25 repeat the same code snippet, with only the filename and variable name changed in each cell. I recommend replacing this repeated pattern with a for
loop mocked below in pseudocode:
files = glob.glob("../../src/tests/data/Slope-20191227T*.optimal.fits")
spectra = [] for file in files: spectrum = HPFSpectrum(file=file, order=19) shifted_spectrum = spectrum.normalize().sky_subtract().remove_nans().blaze_divide_spline().shift_spec(k2_100RV) spectra.append(shifted_spectrum)
If you wish, you may [unpack](https://www.pythontutorial.net/python-basics/python-unpack-list/) those spectra later:
`spec1, spec2, spec3, spec4, spec5 = spectra`. Notice also that your current use of [glob](https://docs.python.org/3/library/glob.html) does not function as a path expander since no wildcards `*` are passed in. I recommend studying the `glob` demo I provide above. But if you could alternatively compose the directory path and filenames manually:
directory_path = ''../../src/tests/data/" filenames = ['Slope-20191227T061633_R01_0011.optimal.fits, Slope-20191227T062631_R01_0012.optimal.fits', ...]
and then later you may prepend the path: `HPFSpectrum(file= directory_path+filename, order=19) `
6. Overplotting with muler
To overplot with muler you can simply pass in a Matplotlib `axes` instance, so in your cell 27, consider replacing with:
shifted_spectrum_3.normalize.plot(ax=ax, label='0013')
7. For loops
Cell 31 can again be replaced with a `for` loop.
8. Barycentric Correction and the need (or not) for resampling
I notice no barycentric correction is currently applied. Are these spectra taken close enough in time that a barycentric correction is negligible? If Barycentric correction is negligible, then could the resampling step in Cell 31 be ignored? If you can ignore the resampling step, the error propagation becomes much easier:
`spec1 + spec2 + ... = spec_net` will automatically yield the correct error propagation.
9. Hysteresis in code execution
It appears that cell 34 has some memory of its execution order, which is not the expected behavior. Let's look into why the execution order matters and try to fix it. Maybe there is an in place operation somewhere, where it is not expected.
10. Explanatory material
The notebook contains a healthy dose of explanatory material in markdown cells. I recommend adding an introductory statement at the top of the notebook below the title and in the same cell as the title. Perhaps start with a motivating statement:
> "Often in astronomy we obtain many snapshots of the same target back-to-back so that individual exposures do not saturate, for cosmic ray mitigation, or other purposes. In this notebook we illustrate how..."
Also, I recommend adding a byline! Below the title and before this introductory statement, put your name, and possibly the Month and Year.
Okay, thank you again for contributing this notebook! I look forward to seeing the codebase grow from contributions like yours.
I am inclined to close this pull request due to inactivity and some recent updates to muler
, including this tutorial which achieves a similar aim as this one. I am happy to reopen the PR, or start a new one to expand our growing collection of tutorials. Thank you @joelburke02, stay tuned to new updates including our awesome new tutorial gallery!
New file that shows an example of how to combine spectra.