Conversation
…the Sphinx configuration files. Added a new glossary section to replace the removed content.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #304 +/- ##
=======================================
Coverage 90.45% 90.45%
=======================================
Files 16 16
Lines 1855 1855
=======================================
Hits 1678 1678
Misses 177 177 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
kelle
left a comment
There was a problem hiding this comment.
Some minor comments, but overall looks extremely useful!!!
weaverba137
left a comment
There was a problem hiding this comment.
This is a great document, and most of my comments are minor. However, I have one major concern: I don't think the Terminology document (terms.rst) should be eliminated unless we are completely satisfied that:
- All information contained in
terms.rsthas been migrated toglossary.rst. terms.rstalso functioned as a thesaurus to some degree, so we should double-check that all items inglossary.rstinclude synonyms and antonyms. Some entries already do, but this should be verified systematically.
Finally, if we do eliminate terms.rst can we provide a redirect to glossary.rst? Alternatively, keep the existing terms.rst, mark it as deprecated, and link to glossary.rst.
docs/glossary.rst
Outdated
| light from one or more sources. One axis corresponds to wavelength (the | ||
| :term:`dispersion axis`) and the other to spatial position along the slit | ||
| (the :term:`cross-dispersion axis`). Also called a spectral image. This is | ||
| the input to the extraction process that produces a :term:`1D spectrum`. |
There was a problem hiding this comment.
I would add a qualifier here that 2D Spectra are not unique to slit spectroscopy, fiber spectrographs start with 2d images as well.
There was a problem hiding this comment.
You could also reference MOS (multi-object spectroscopy) here.
There was a problem hiding this comment.
I've revised the 2D Spectrum definition to include the qualifier, added a link to MOS, and also added a definition for "Fiber-fed spectroscopy".
docs/glossary.rst
Outdated
| A three-dimensional data structure with two spatial dimensions and one | ||
| spectral dimension. Data cubes are the standard output format for | ||
| :term:`IFU` observations, where each spatial pixel contains a complete | ||
| spectrum. Also called a spectral data cube or hyperspectral cube. |
There was a problem hiding this comment.
You might also want to include the term "spaxel".
docs/glossary.rst
Outdated
| different wavelengths falls on different positions along this axis. | ||
|
|
||
| Emission Line | ||
| A bright, narrow feature in a spectrum at a specific wavelength, |
There was a problem hiding this comment.
I wouldn't use the term "narrow" without some qualification. For example, QSO emission lines can be very broad. In fact, "bright" could also be a bit vague. Maybe "An excess flux feature in a spectrum at a specific wavelength, ...". I'd be open to any alternate wording here.
There was a problem hiding this comment.
Thanks, I've revised this now.
docs/glossary.rst
Outdated
| FWHM | ||
| Full Width at Half Maximum. A measure of the width of a peak or line | ||
| profile, defined as the width at which the intensity drops to half its | ||
| maximum value. FWHM is commonly used to characterize :term:`spectral |
There was a problem hiding this comment.
Absorption lines also have a FWHM, so "drops to half" isn't quite right here. You could do something simple like "width at which the intensity drops (rises) to half its maximum (minimum) value.
There was a problem hiding this comment.
Take a look at the revised version. I've also added a definition for "Absorption line".
docs/glossary.rst
Outdated
| A spectrum that has been resampled so that the :term:`dispersion axis` | ||
| is aligned with image rows or columns, with constant wavelength spacing. | ||
| Rectification involves :term:`resampling`. A rectified | ||
| ND spectrum has one axis that is purely spectral. |
There was a problem hiding this comment.
I used ND originally to include 3D data cubes, etc. I've changed this now to 2D for simplicity.
I chose the third option. The original |
weaverba137
left a comment
There was a problem hiding this comment.
I had two minor comments on this round. Looking good overall.
docs/glossary.rst
Outdated
| :term:`barycentric correction`, which provides higher precision. | ||
|
|
||
| IFU | ||
| `Integral Field Unit <https://en.wikipedia.org/wiki/Integral_field_spectrograph>`__. |
There was a problem hiding this comment.
This might not make any practical difference, but this external hyperlink is terminated with two underscores, while other external hyperlinks have one underscore.
There was a problem hiding this comment.
Thanks for catching this; it was a typo.
docs/glossary.rst
Outdated
|
|
||
| Spectrum | ||
| The distribution of light intensity as a function of wavelength or | ||
| frequency. In specreduce and specutils, ``Spectrum`` specifically |
There was a problem hiding this comment.
I would make this a link to specutils.Spectrum.
This PR addresses #285 by replacing the old
terms.rstdocument with a full glossary. Please go through it and let me know if you disagree with any of the definitions or if you think anything is missing. You can find a rendered version of the glossary at https://specreduce--304.org.readthedocs.build/en/304/glossary.html.LLM disclaimer: This PR includes rather (very) heavy use of LLMs, and I'd like to hear comments from people who might not be comfortable with this (comments from people who are comfortable with this are, of course, also welcome ;) ). My workflow was:
terms.rstSo, the glossary is based on the existing terminology document, my own knowledge, and three (paid) LLMs. The input from the LLMs is substantial, but I have gone through the final set of definitions several times to ensure that there is nothing I would consider wrong (and likely added some typos along the way).