Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor nexus writer #306

Merged

Conversation

Modularius
Copy link
Contributor

Summary of changes

  • Added thiserror style error handling removing over-reliance on anyhow. The code now conforms to ADR 7. Standardise error handling.
  • Added traits to extend functionality for hdf5::{Group, Dataset, Attribute} in hdf5_writer.rs.
  • Error handling includes a hdf5 path where appropriate to enable better locality, and a Location field in non-hdf5 related errors.
  • More standardised handling of runlogs/selogs/alarms. All logs contain a time Dataset with a "units" and "start" attribute, where "start" denotes the RFC3999 date/time to which the time value is relative to.

Instruction for review/testing

  • General code review.
  • Has been tested on simulated data as well as on live HiFi data.

@Modularius Modularius requested a review from DanNixon January 31, 2025 14:16
Copy link
Member

@DanNixon DanNixon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

A couple of other things that might as well be done now:

  • Could the term "run file" be revisited. This does not really seem to describe the thing it actually refers to (a separate file describing a run vs. a collection of run metadata).
  • Could crate::hdf5_file::run_file_components be moved to crate::hdf5_file::run_file::components?

@Modularius
Copy link
Contributor Author

The RunFile struct does contain the hdf5::File struct as well as the RunFileContents struct abstracting the file's contents. You might be thinking of the RunParameters which does largely just contain the metadata.

This PR is the first in a couple I've got planned which will, among other things make the naming conventions more logical. It's probably okay to leave run_file_components as is for now, as it might not exist in the next one.

@Modularius Modularius requested a review from DanNixon February 5, 2025 14:18
Copy link
Member

@DanNixon DanNixon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@Modularius Modularius requested a review from DanNixon February 14, 2025 11:53
@DanNixon DanNixon merged commit 32ea120 into STFC-ICD-Research-and-Design:main Feb 17, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants