Add the ability to save a file that can be read by Mantid by AnthonyLim23 · Pull Request #108 · ISISMuon/MNeuEventLib

AnthonyLim23 · 2026-09-02T16:28:19Z

This PR adds code to clean up the current data (should not have null values in the event file) and add the missing meta-data so it can be loaded into Mantid.

The Python script I wrote for creating the reference file has been added under tools, for completeness.

The code that cleans and adds meta-data to the output is in Rust. At present it does not handle Dwell and this will be fixed as part of #77.

I will check the output against the schema and add the code needed to output a json of the missing/incorrect data as part of #109.

I have the following left:

Add example to docs
Add Mantid system test
Clean up the code

To test:

from MNeuEventLib import Data
file = 'HIFI00207745_events.nxs'

data = Data(file, 64)
result = data.calculate()
data.save_nexus('HIFI42.nxs')

Then check HIFI42.nxs loads in both Mantid's muon analysis GUI and Wimda.

alexhroom

very cursory initial look - i'll test properly once you've added system tests.

as a general note, the docstring style is inconsistent: in some places it has

Parameters
----------

and in some places it has

## Parameters

both of which are not consistent to the docstring style used in the rest of the code (numpy style)

alexhroom · 2026-09-03T13:42:46Z

can this file start with a docstring/comment explaining what it does and what it's for?

alexhroom · 2026-09-03T13:43:45Z

+
+def test_Mantid_install():
+    """
+    Lets first test that Mantid works


Suggested change

Lets first test that Mantid works

Test that Mantid has correctly installed.

Function docstrings should be imperatives.

github-actions · 2026-09-04T13:52:26Z

👋 Hi, @AnthonyLim23,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.

This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

AnthonyLim23 · 2026-09-10T10:51:14Z

I still need to update the docs and mantid test

alexhroom · 2026-09-14T17:13:51Z

@@ -0,0 +1,3 @@
+//! Code for saving outputs to file.


could this be more specific?

alexhroom · 2026-09-14T17:18:01Z

+                continue;
+            } else if let Ok(_dataset) = sample.dataset(&name) {
+                if name == "thickness" {
+                    let default = Array1::from_shape_vec(1, vec![0.0f32]).unwrap();


Suggested change

let default = Array1::from_shape_vec(1, vec![0.0f32]).unwrap();

let default = Array1::from_shape_vec(1, vec![0.0f32])?;

replace unwraps with ? in functions that return Result

alexhroom · 2026-09-14T17:18:44Z

+        create_default_dataset(&source_parent.group(name).unwrap(), dest, name, shapes)?;
+        Ok(())
+    } else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_ok() {
+        // if dataset exists in both files
+        println!("{} dataset already exists", name);
+        let src_ds = source_parent.dataset(name)?;
+        let dst_ds = dest.dataset(name)?;
+        for att_name in src_ds.attr_names()? {
+            println!("copy attribute {}", att_name);
+            copy_attr(&src_ds, &dst_ds, &att_name)?;
+        }
+        Ok(())
+    } else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_err() {
+        // if dataset exists in source but not in destination
+        println!("copy dataset {}", name);
+        source_parent.dataset(name)?.copy_to(dest, name)?;
+        Ok(())
+    } else if source_parent.group(name).is_ok() && dest.group(name).is_ok() {
+        // if group exists in both files
+        println!("group {} exists in both files, going deeper", name);
+        for member in source_parent.group(name).unwrap().member_names()? {
+            set_defaults(
+                &source_parent.group(name).unwrap(),
+                &dest.group(name).unwrap(),
+                member.as_str(),


Suggested change

create_default_dataset(&source_parent.group(name).unwrap(), dest, name, shapes)?;

Ok(())

} else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_ok() {

// if dataset exists in both files

println!("{} dataset already exists", name);

let src_ds = source_parent.dataset(name)?;

let dst_ds = dest.dataset(name)?;

for att_name in src_ds.attr_names()? {

println!("copy attribute {}", att_name);

copy_attr(&src_ds, &dst_ds, &att_name)?;

}

Ok(())

} else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_err() {

// if dataset exists in source but not in destination

println!("copy dataset {}", name);

source_parent.dataset(name)?.copy_to(dest, name)?;

Ok(())

} else if source_parent.group(name).is_ok() && dest.group(name).is_ok() {

// if group exists in both files

println!("group {} exists in both files, going deeper", name);

for member in source_parent.group(name).unwrap().member_names()? {

set_defaults(

&source_parent.group(name).unwrap(),

&dest.group(name).unwrap(),

member.as_str(),

create_default_dataset(&source_parent.group(name)?, dest, name, shapes)?;

Ok(())

} else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_ok() {

// if dataset exists in both files

println!("{} dataset already exists", name);

let src_ds = source_parent.dataset(name)?;

let dst_ds = dest.dataset(name)?;

for att_name in src_ds.attr_names()? {

println!("copy attribute {}", att_name);

copy_attr(&src_ds, &dst_ds, &att_name)?;

}

Ok(())

} else if source_parent.dataset(name).is_ok() && dest.dataset(name).is_err() {

// if dataset exists in source but not in destination

println!("copy dataset {}", name);

source_parent.dataset(name)?.copy_to(dest, name)?;

Ok(())

} else if source_parent.group(name).is_ok() && dest.group(name).is_ok() {

// if group exists in both files

println!("group {} exists in both files, going deeper", name);

for member in source_parent.group(name)?.member_names()? {

set_defaults(

&source_parent.group(name)?,

&dest.group(name)?,

member.as_str(),

replace unwraps with ? in functions that return Result

alexhroom · 2026-09-14T17:19:43Z

+    let name = default.name().split("dataset_").last().unwrap().to_string();
+    let key: String = default
+        .dataset("shape")
+        .unwrap()
+        .read_scalar::<hdf5::types::VarLenUnicode>()
+        .unwrap()
+        .as_str()
+        .to_string();
+    let len: usize = *shapes.get(&key).unwrap();
+    let dtype: &hdf5::types::VarLenUnicode =
+        &default.dataset("dtype").unwrap().read_scalar().unwrap();
+    if dtype.as_str() == "int32" {
+        let default_value = default.dataset("default").unwrap().read_scalar::<i32>()?;
+        if let Err(e) = add_array(dest, &Array1::from_elem(len, default_value), &name) {
+            eprintln!("error: {e}");
+            eprintln!("chain: {e:?}");
+        };
+    } else if dtype.as_str() == "float32" {
+        let default_value = default.dataset("default").unwrap().read_scalar::<f32>()?;
+        add_array(dest, &Array1::from_elem(len, default_value), &name)?;
+    } else if dtype.as_str() == "float64" {
+        let default_value = default.dataset("default").unwrap().read_scalar::<f64>()?;


Suggested change

let name = default.name().split("dataset_").last().unwrap().to_string();

let key: String = default

.dataset("shape")

.unwrap()

.read_scalar::<hdf5::types::VarLenUnicode>()

.unwrap()

.as_str()

.to_string();

let len: usize = *shapes.get(&key).unwrap();

let dtype: &hdf5::types::VarLenUnicode =

&default.dataset("dtype").unwrap().read_scalar().unwrap();

if dtype.as_str() == "int32" {

let default_value = default.dataset("default").unwrap().read_scalar::<i32>()?;

if let Err(e) = add_array(dest, &Array1::from_elem(len, default_value), &name) {

eprintln!("error: {e}");

eprintln!("chain: {e:?}");

};

} else if dtype.as_str() == "float32" {

let default_value = default.dataset("default").unwrap().read_scalar::<f32>()?;

add_array(dest, &Array1::from_elem(len, default_value), &name)?;

} else if dtype.as_str() == "float64" {

let default_value = default.dataset("default").unwrap().read_scalar::<f64>()?;

let name = default.name().split("dataset_").last()?.to_string();

let key: String = default

.dataset("shape")?

.read_scalar::<hdf5::types::VarLenUnicode>()?

.as_str()

.to_string();

let len: usize = *shapes.get(&key)?;

let dtype: &hdf5::types::VarLenUnicode =

&default.dataset("dtype")?.read_scalar()?;

if dtype.as_str() == "int32" {

let default_value = default.dataset("default")?.read_scalar::<i32>()?;

if let Err(e) = add_array(dest, &Array1::from_elem(len, default_value), &name) {

eprintln!("error: {e}");

eprintln!("chain: {e:?}");

};

} else if dtype.as_str() == "float32" {

let default_value = default.dataset("default")?.read_scalar::<f32>()?;

add_array(dest, &Array1::from_elem(len, default_value), &name)?;

} else if dtype.as_str() == "float64" {

let default_value = default.dataset("default")?.read_scalar::<f64>()?;

replace unwraps with ? in functions that return Result

alexhroom · 2026-09-14T17:20:15Z

+        println!("Attribute '{name}' already exists in destination dataset, skipping");
+        return Ok(());
+    }
+    let cname = CString::new(name).unwrap();


Suggested change

let cname = CString::new(name).unwrap();

let cname = CString::new(name)?;

replace unwraps with ? in functions that return Result

alexhroom · 2026-09-14T17:32:44Z

+this includes the file path), then when saving 
+event data to histograms, the command would be
+
+data.save(<output>, autofill=Ture, ref_file=<ref>)


Suggested change

data.save(<output>, autofill=Ture, ref_file=<ref>)

data.save(<output>, autofill=True, ref_file=<ref>)

alexhroom · 2026-09-14T17:42:10Z

+///    The destination group
+/// name: &str
+///    The name of the attribute to copy
+unsafe fn _copy_attr(src: &Location, dst: &Location, name: &str) -> Result<()> {


could you add some code comments explaining why using the low-level bindings is necessary in each case?

alexhroom · 2026-09-14T17:46:07Z

+    new_ds.write(new_data)?;
+
+    for attr_name in &attr_names {
+        let _ = copy_attr(&old, &new_ds, attr_name);


Suggested change

let _ = copy_attr(&old, &new_ds, attr_name);

copy_attr(&old, &new_ds, attr_name)?;

using let _ ignores the error if copy_attr returns an error

alexhroom · 2026-09-14T17:53:07Z

+/// ----------
+/// file_name: &str
+///     The name of the file to get the period information from
+pub fn get_p_info(file_name: &str) -> Result<(usize, usize)> {


change name to get_period_info

alexhroom · 2026-09-14T17:54:03Z

+    ///     Contains "correct" data that should be copied to the output file.
+    ///     This is only need it the reference file needed is not the standard
+    ///     muon nexus v2 file. The ref_file is generated from tools/make_default.py.
+    pub fn save_nexus(&self, filename: String, ref_file: String) -> Result<()> {


what is this function actually doing? it doesn't seem to actually save any of the data, just save the default values to an already-created data file. maybe it needs a better name and docstring?

also does it need to be in the python API? will a user ever use this directly or is it only used through save?

AnthonyLim23 added 11 commits September 1, 2026 12:06

works with mantid for p >1

6a8c376

update save.py

caf1c0f

fix conflict

015f263

refs #62 added more code to clean file

bcab619

refs #62 can load into mantid

26d01a6

add script

3d121d6

refs #62 can load into mantid

92e5cc3

clean up code and added some tests

68cd6c2

fixed some failing tests

8b13a52

fix unit tests

eeaea52

fix linting

d43d52d

AnthonyLim23 added feature New feature or request data Concerns handling data input/output labels Sep 2, 2026

AnthonyLim23 added 3 commits September 2, 2026 17:29

fix linting

5922ebf

code clean up

7567d09

Merge remote-tracking branch 'origin/main' into 62_save_3

c1923ac

alexhroom requested changes Sep 3, 2026

View reviewed changes

AnthonyLim23 force-pushed the 62_save_3 branch from eeb5d75 to c1923ac Compare September 4, 2026 13:13

add integration tests to action

228bc01

github-actions Bot added the Has Conflicts This PR has a merge conflict. label Sep 4, 2026

Merge branch 'main' into 62_save_3

cb89ad9

github-actions Bot removed the Has Conflicts This PR has a merge conflict. label Sep 8, 2026

AnthonyLim23 added 7 commits September 8, 2026 15:53

some fixes after merge - more testing needed

d245743

make it work for batch

8a7efea

add integration tests for mantid

f4a1f9d

fix formating mistake in test_mantid

303c36f

fix typo in test_mantid

0f44de8

fix file path in test_mantid

f108254

fix missing step in test_mantid

215f3e6

AnthonyLim23 added 3 commits September 9, 2026 17:07

fix wrong file path in test_mantid

b953225

deal with different number of return values in test_mantid

dbf9cbf

missing deadtime table in test_mantid

29cd597

AnthonyLim23 commented Sep 10, 2026

View reviewed changes

Comment thread src/data/save/wimda.rs

AnthonyLim23 added 3 commits September 10, 2026 11:34

address review comments

8be746e

fix checks

134f46e

fix checks

88e6cbc

AnthonyLim23 added 8 commits September 14, 2026 14:05

add single period tests

7f97f91

fix bug in integration tests

f4e3837

fix period bug in integration tests

fd942de

fix another bug in integration tests

8f257e2

fix access bug in integration tests

2529aa4

fix file name bug in integration tests

554c071

fix file name bug in integration tests

6c5ca51

fix access bug in integration tests

e4e455c

alexhroom force-pushed the 62_save_3 branch from 1d93326 to e4e455c Compare September 14, 2026 17:03

alexhroom requested changes Sep 14, 2026

View reviewed changes

	Lets first test that Mantid works
	Test that Mantid has correctly installed.

	let default = Array1::from_shape_vec(1, vec![0.0f32]).unwrap();
	let default = Array1::from_shape_vec(1, vec![0.0f32])?;

	let cname = CString::new(name).unwrap();
	let cname = CString::new(name)?;

	data.save(<output>, autofill=Ture, ref_file=<ref>)
	data.save(<output>, autofill=True, ref_file=<ref>)

	let _ = copy_attr(&old, &new_ds, attr_name);
	copy_attr(&old, &new_ds, attr_name)?;

Uh oh!

Conversation

AnthonyLim23 commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexhroom left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 4, 2026

Uh oh!

Uh oh!

AnthonyLim23 commented Sep 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AnthonyLim23 commented Sep 2, 2026 •

edited

Loading