Skip to content

Add documentation of reduction interface and allow skipping file saving.#172

Merged
YooSunYoung merged 14 commits intotof-sim-configfrom
documentation
Dec 16, 2025
Merged

Add documentation of reduction interface and allow skipping file saving.#172
YooSunYoung merged 14 commits intotof-sim-configfrom
documentation

Conversation

@YooSunYoung
Copy link
Copy Markdown
Member

No description provided.

return [self.inputs, self.workflow, self.output]


def to_command_arguments(
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I moved this interface to the configurations module so that users can use it.

Comment thread src/ess/nmx/nexus.py

"""
_check_file(output_file, overwrite=True)
_check_file(output_file, overwrite=overwrite)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Rest of the nxlauetof writing is done in the append mode to make sure they are called on top of this function.

Comment thread docs/user-guide/workflow.ipynb Outdated
Comment thread docs/user-guide/workflow.ipynb Outdated
Comment thread docs/user-guide/workflow.ipynb Outdated
Comment thread docs/user-guide/workflow.ipynb Outdated
"The `essnmx-reduce` interface will reduce `nexus` file <br>\n",
"and save the results into `NXlauetof`(not exactly but very close) format for `dials`.<br>\n",
"\n",
"Argument options could be exhaustive therefore we wrapped them into a nested pydantic model.<br>\n",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"Argument options could be exhaustive"

Not sure I understand what is meant here...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I meant, if we pass them as individual arguments to the function... it could be exhaustive and not so easy to know which one to change etc. We can change it to ... For conveniences and safety, we wrapped...

Comment thread docs/user-guide/workflow.ipynb Outdated
Comment thread docs/user-guide/workflow.ipynb Outdated
" input_file=[\"PATH_TO_THE_NEXUS_FILE.hdf\"],\n",
" detector_ids=[0, 1, 2], # Detector index to be reduced in alphabetical order.\n",
" ),\n",
" output=OutputConfig(output_file=\"scipp_output.hdf\", skip_file_output=True),\n",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Instead of having both output_file and skip_file_output, can we say that if output_file is None, we skip file output? I don't know if that is better API, but it feels like if we are skipping file output, then we shouldn't need to define output_file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's because there is already a default output file name, which is not None.
For users, or developers, it'll be easier to figure out how to skip writing a file if there's a dedicated flag, rather than having to know you should set the output file to None.

What do you think...?

Co-authored-by: Neil Vaytet <39047984+nvaytet@users.noreply.github.com>
Comment thread docs/user-guide/workflow.ipynb Outdated
YooSunYoung and others added 3 commits December 16, 2025 14:03
Comment thread docs/user-guide/workflow.ipynb Outdated
@YooSunYoung YooSunYoung merged commit ce62e69 into tof-sim-config Dec 16, 2025
@YooSunYoung YooSunYoung deleted the documentation branch December 16, 2025 15:05
@github-project-automation github-project-automation Bot moved this from In progress to Done in Development Board Dec 16, 2025
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