Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions src/pyfaradaycup/decommutator/README.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Decommutator

This directory contains the code that converts Parker Solar Probe SPC L0 files into L1 CDF files.
This directory contains the code that converts Parker Solar Probe SPC L0 files into L0.5 CDF files.
For historical reasons, the output files are named `*_L1.cdf`.

- `main.py` defines the `pfc_decommutator` command line tool.
- `swp_spc_l0_to_l05.py` defines `main`, which does the conversion for one L0 file.
Expand All @@ -19,21 +20,22 @@ The changes below are listed with the most important first.
Dagster decides whether a step succeeded from the exit code of a subprocess, or from whether a Python call raised an exception.

- **Stop skipping APIDs that fail.**
When a skeleton file is missing, or when writing a CDF fails, `main` logs a message and moves to the next APID, and the run still ends with "Script complete".
When a skeleton file is missing, when a CDF cannot be created or filled, or when the APID is 0x07B, `main` logs a message and moves to the next APID, and the run still ends with "Script complete".
A CDF that could not be filled is left behind without being closed.
Collect these failures and raise an exception at the end if there were any.

### Return what was written

`swp_spc_l02l1.main` returns `None`.
It should return the paths of the L1 CDF files it wrote, and the path of the log file, so that a Dagster asset can check its outputs and attach them as metadata without searching the output directory.
`swp_spc_l0_to_l05.main` returns `None`.
It should return the paths of the L0.5 CDF files it wrote, and the path of the log file, so that a Dagster asset can check its outputs and attach them as metadata without searching the output directory.
It would also be useful to return the kernels and skeleton files that were used.

### Make retries and parallel runs safe

- **Write each CDF to a temporary file, and rename it when it is complete.**
CDF files are currently written in place, so a run that stops part way leaves an incomplete CDF file behind.
- **Decide what a retry should do.**
Without `--overwrite`, a retry stops at the first L1 file that already exists.
Without `--overwrite`, a retry stops at the first L0.5 CDF file that already exists.
A Dagster step should either always overwrite, or skip files that are already complete.
- **Give each run its own log file name.**
The log file name contains the time to the nearest second, and the file is opened for writing.
Expand All @@ -43,14 +45,16 @@ It would also be useful to return the kernels and skeleton files that were used.
The log file is the module-level global `logfile`, which `_statusmsg` writes to.
SPICE kernels are loaded with `spiceypy.furnsh` on every call and are never unloaded.
This is safe when each step runs in its own process, but not when `main` is called more than once in the same process.
Pass the log file as an argument, and unload the kernels when the conversion finishes.
The log file is also left open when `main` raises an exception.
Pass the log file as an argument, close it in a `finally` block or with a `with` statement, and unload the kernels when the conversion finishes.

### Make it callable from Python

Calling the conversion as a function is better suited to Dagster than starting a subprocess, since Dagster can then capture exceptions, log messages, and return values.

- **Use the `logging` module instead of `_statusmsg` and `print`.**
- **Use the `logging` module instead of `_statusmsg`, `print`, and `sys.stdout.write`.**
Dagster can then show the messages with their levels in its own logs.
`_read_file` and `_read_file_sc` write their progress and a packet summary to standard output even without `--verbose`.
The warnings that `pfc_decommutator` prints for options that have no effect should use `logging` too.
- **Keep `pfc_decommutator` as a thin wrapper.**
A click command ends the Python process when it finishes, so it should not be called from an asset or op.
Expand All @@ -68,8 +72,8 @@ Calling the conversion as a function is better suited to Dagster than starting a
### Test the `--spacecraft` option

- **Add a test that converts a spacecraft L0 file with `--spacecraft`.**
The only test converts an instrument L0 file, so `_read_file_sc`, `_get_layout_sc`, and the code that writes the spacecraft housekeeping CDFs are never run.
Add a spacecraft L0 file to `tests/data`, along with the expected L1 CDF files, and compare them as the existing test does.
The only end-to-end test converts an instrument L0 file, so `_read_file_sc`, `_get_layout_sc`, and the code that writes the spacecraft housekeeping CDFs are never run.
Add a spacecraft L0 file to `tests/data`, along with the expected L0.5 CDF files, and compare them as the existing test does.
Include a file for which the flight software version cannot be found from the packet header, so that the fallback to the file path, and then to the newest definition file, is also tested.

### Remove options that do nothing
Expand Down
12 changes: 6 additions & 6 deletions src/pyfaradaycup/decommutator/main.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Command line tool for converting SPC L0 files into L1 CDF files."""
"""Command line tool for converting SPC L0 files into L0.5 CDF files."""

from __future__ import annotations

Expand Down Expand Up @@ -98,8 +98,8 @@ def _warn_about_ineffective_options(
"--overwrite",
is_flag=True,
help=(
"Overwrite existing L1 CDF files. Without this, the tool exits "
"if an L1 CDF file already exists."
"Overwrite existing L0.5 CDF files. Without this, the tool exits "
"if an L0.5 CDF file already exists."
),
)
@click.option(
Expand All @@ -117,7 +117,7 @@ def _warn_about_ineffective_options(
default="0",
show_default=True,
callback=_parse_apid,
help="APID to create L1 file for, in hexadecimal (e.g., 35E) [0==all]",
help="APID to create L0.5 CDF file for, in hexadecimal (e.g., 35E) [0==all]",
)
@click.option(
"-l0",
Expand Down Expand Up @@ -163,7 +163,7 @@ def pfc_decommutator( # ruff:ignore[PLR0913]
logdir: pathlib.Path | None,
) -> None:
"""
Convert one SPC L0 file into L1 CDF files, one per APID.
Convert one SPC L0 file into L0.5 CDF files, one per APID.

To convert the L0 file 0523462910_4_EA, printing messages to the
screen as well as to the log file:
Expand All @@ -175,7 +175,7 @@ def pfc_decommutator( # ruff:ignore[PLR0913]
--logdir=/path/to/logdir \\
-v

This writes one L1 CDF file for each APID found in the L0 file, such
This writes one L0.5 CDF file for each APID found in the L0 file, such
as 0523462910_4_EA_APID351_L1.cdf, into the directory given by
--l05dir, and a log file into the directory given by --logdir. If
--l05dir or --logdir is not given, the current directory is used.
Expand Down
10 changes: 5 additions & 5 deletions src/pyfaradaycup/decommutator/swp_spc_l0_to_l05.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915]
nowdt = datetime.datetime.now() # ruff:ignore[DTZ005]
logpath = (
logdir
/ f"swp_spc_l02l1_{nowdt.year:04.0f}{nowdt.month:02.0f}{nowdt.day:02.0f}{nowdt.hour:02.0f}{nowdt.minute:02.0f}{nowdt.second:02.0f}.log"
/ f"swp_spc_l0_to_l05_{nowdt.year:04.0f}{nowdt.month:02.0f}{nowdt.day:02.0f}{nowdt.hour:02.0f}{nowdt.minute:02.0f}{nowdt.second:02.0f}.log"
)
try:
logdir.mkdir(parents=True, exist_ok=True)
Expand Down Expand Up @@ -230,7 +230,7 @@ def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915]

if apid == 0x07B: # ruff:ignore[PLR2004]
_statusmsg(
"***WARNING*** [swp_spc_l02l1] APID 0x07B CDFs not yet implemented",
"***WARNING*** [swp_spc_l0_to_l05] APID 0x07B CDFs not yet implemented",
screen=True,
verbose=verbose,
)
Expand Down Expand Up @@ -279,7 +279,7 @@ def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915]

# if we get here, this file already exists; so delete it, if desired
_statusmsg(
f"***INFO*** [swp_spc_l02l1] L0.5 CDF file ({l1path}) already exists",
f"***INFO*** [swp_spc_l0_to_l05] L0.5 CDF file ({l1path}) already exists",
screen=True,
verbose=verbose,
)
Expand Down Expand Up @@ -361,7 +361,7 @@ def main( # ruff:ignore[C901, PLR0912, PLR0913, PLR0915]
cdf.close()

_statusmsg(
"***INFO*** [swp_spc_l02l1] Script complete.", screen=True, verbose=verbose
"***INFO*** [swp_spc_l0_to_l05] Script complete.", screen=True, verbose=verbose
)

# Close the log file
Expand Down Expand Up @@ -561,7 +561,7 @@ def _cdf351_353_354( # ruff:ignore[C901, PLR0912, PLR0915, RET503]

if not itst_warned:
_statusmsg(
"***WARNING*** [swp_spc_l02l1] The reported IT+ST is not an even power of 2. Using IT=6,ST=2...",
"***WARNING*** [swp_spc_l0_to_l05] The reported IT+ST is not an even power of 2. Using IT=6,ST=2...",
screen=True,
verbose=verbose,
)
Expand Down
Loading