Auto-generate tables for the wavelength calibration documentation #1802
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #1802 +/- ##
===========================================
- Coverage 38.02% 38.00% -0.03%
===========================================
Files 211 211
Lines 48975 48975
===========================================
- Hits 18625 18614 -11
- Misses 30350 30361 +11 ☔ View full report in Codecov by Sentry. |
modified: doc/scripts/build_wvcal_tbl_rst.py
There was a problem hiding this comment.
Nice and very needed PR. Am approving, but think about this issue:
My main concern is over maintaining the summary table of information about the reid_arxiv files. I would suggest adding code in the main repo that includes this information in the reid_arxiv files when created, and otherwise hand-adding the same to all the existing files. In this way, no one would need to maintain the summary.txt file, and the .rst table would be automagically generated each time with the files in the directory.
There was a problem hiding this comment.
It's going to be a bear of a job filling this in the first time. Can't we pull the instrument and disperser from the files when building this table in the first place?
I like the checking logic added to build_wvcal_tbl_rst.py to ensure a 1:1 match between the directory listing and this file, but I worry that the maintenance of this monstrosity will fall to @kbwestfall when he preps the docs for each release.
While it may be a bit of a headache to do, could we add the necessary metadata to each reid_arxiv file (by hand, if necessary, but automatically for any new ones) so that this table can be generated automagically?
|
|
||
| def write_reid_arxiv_table(output_root): | ||
|
|
||
| # TODO: Pull wavelength range (and resolution?) from files |
There was a problem hiding this comment.
See comment about this on the summary.txt file.
| # Check that all the files are in the table and vice versa | ||
| names = numpy.array([f.name for f in files]) | ||
| indx = numpy.isin(names, tbl['File']) | ||
| if not numpy.all(indx): | ||
| raise ValueError('Files found in reid_arxiv directory that are *not* in the summary.txt ' | ||
| f'file. Include summary entries for the following (or delete them): ' | ||
| f'{names[numpy.logical_not(indx)]}') | ||
| indx = numpy.isin(tbl['File'], names) | ||
| if not numpy.all(indx): | ||
| raise ValueError('Files found in the summary.txt file that are *not* in the reid_arxiv ' | ||
| 'directory. Remove summary entries for the following: ' | ||
| f'{tbl["File"][numpy.logical_not(indx)]}') |
Merges develop into release (1.17.0 tag prep)
As titled.
This is largely a documentation PR. There are some changes to how
datetimeis imported because I realized thatdatetime.datetime.UTChas never existed (butdatetime.UTCdoes), meaning the try blocks used to import__UTC__were always faulting (not just for python<=3.10).As a team, we need to fill in the new
pypeit/data/arc_lines/reid_arxiv/summary.txtfile!