Skip to content

Allow Nexus to make nested groups with same name #39686

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 22, 2025
Merged

Allow Nexus to make nested groups with same name #39686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3141825
add check for data
searscr Jul 17, 2025
aaebbcf
release note and fix
searscr Jul 18, 2025
27dbca5
cppcheck
searscr Jul 18, 2025
3479538
Remove release notes
thomashampson Jul 18, 2025
37b2b40
change assert type
searscr Jul 21, 2025
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
2 changes: 2 additions & 0 deletions Framework/DataHandling/inc/MantidDataHandling/SaveNXSPE.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ class MANTID_DATAHANDLING_DLL SaveNXSPE final : public API::Algorithm {
void init() override;
/// Execution code
void exec() override;
/// Validate Inputs code
std::map<std::string, std::string> validateInputs() override;

// Some constants to be written for masked values.
/// Value for data if pixel is masked
Expand Down
9 changes: 9 additions & 0 deletions Framework/DataHandling/src/SaveNXSPE.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ void SaveNXSPE::init() {
See [[FindDetectorsPar]] description for the details.");
}

std::map<std::string, std::string> SaveNXSPE::validateInputs() {
std::map<std::string, std::string> issues;
std::string inputWS = getPropertyValue("InputWorkspace");
if (inputWS.find("/") != std::string::npos) {
issues["InputWorkspace"] = "The input workspace name cannot contain a '/' character.";
}
return issues;
}

/**
* Execute the algorithm
*/
Expand Down
32 changes: 32 additions & 0 deletions Framework/DataHandling/test/SaveNXSPETest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <cxxtest/TestSuite.h>

#include "MantidAPI/AnalysisDataService.h"
#include "MantidAPI/FrameworkManager.h"
#include "MantidAPI/NumericAxis.h"
#include "MantidDataHandling/LoadInstrument.h"
Expand Down Expand Up @@ -190,6 +191,20 @@ class SaveNXSPETest : public CxxTest::TestSuite {
Poco::File(outputFile).remove();
}

void test_WorkspaceNameData() {
auto saver = setupWithWSName("data");
TS_ASSERT_THROWS_NOTHING(saver->execute());
TS_ASSERT(saver->isExecuted());
}

void test_WorkspaceBadName() {
auto saver = setupWithWSName("bad/name");
TS_ASSERT_THROWS_EQUALS(saver->execute(), const std::runtime_error &e, std::string(e.what()),
"Some invalid Properties found: \n"
" InputWorkspace: The input workspace name cannot contain a \'/\' character.");
TS_ASSERT(!saver->isExecuted());
}

private:
MatrixWorkspace_sptr makeWS_direct(MatrixWorkspace_sptr inputWS, double ei = 12.0) {
std::unique_ptr<Mantid::Kernel::PropertyWithValue<std::string>> mode(
Expand Down Expand Up @@ -305,4 +320,21 @@ class SaveNXSPETest : public CxxTest::TestSuite {

return boost::make_tuple(dims, signal, error, efixed);
}

SaveNXSPE *setupWithWSName(const std::string &workspaceName) {
MatrixWorkspace_sptr input = makeWorkspace();
Mantid::API::AnalysisDataService::Instance().add(workspaceName, input);

auto *saver = new SaveNXSPE();
saver->initialize();
saver->setChild(true);
TS_ASSERT_THROWS_NOTHING(saver->setProperty("InputWorkspace", workspaceName));
TS_ASSERT_EQUALS(saver->getPropertyValue("InputWorkspace"), workspaceName);
std::string outputFile("SaveNXSPETest_testEXEC.nxspe");
TS_ASSERT_THROWS_NOTHING(saver->setPropertyValue("Filename", outputFile));
outputFile = saver->getPropertyValue("Filename"); // get absolute path
TS_ASSERT_THROWS_NOTHING(saver->setProperty("Psi", 0.0));
TS_ASSERT_THROWS_NOTHING(saver->setProperty("KiOverKfScaling", true));
return saver;
}
};
2 changes: 1 addition & 1 deletion Framework/Nexus/inc/MantidNexus/nxstack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void setCloseID(pFileStack self, const NXlink &id);

int fileStackDepth(pFileStack self);

void pushPath(pFileStack self, const char *name);
void pushPath(pFileStack self, const char *name, bool isdata = false);
void popPath(pFileStack self);
int buildPath(pFileStack self, char *path, int pathlen);

Expand Down
2 changes: 1 addition & 1 deletion Framework/Nexus/src/napi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ NXstatus NXopendata(NXhandle fid, CONSTCHAR *name) {
status = LOCKED_CALL(pFunc->nxopendata(pFunc->pNexusData, name));

if (status == NXstatus::NX_OK) {
pushPath(fileStack, name);
pushPath(fileStack, name, true);
}

char nxurl[1024];
Expand Down
4 changes: 2 additions & 2 deletions Framework/Nexus/src/nxstack.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ void setCloseID(pFileStack self, const NXlink &id) { self->fileStack[self->fileS
/*----------------------------------------------------------------------*/
int fileStackDepth(pFileStack self) { return self->fileStackPointer; }
/*----------------------------------------------------------------------*/
void pushPath(pFileStack self, const char *name) {
if (self->pathPointer >= 0 && name == self->pathStack[self->pathPointer]) {
void pushPath(pFileStack self, const char *name, bool isdata) {
if (self->pathPointer >= 0 && name == self->pathStack[self->pathPointer] && isdata) {
return;
}
self->pathPointer++;
Expand Down
14 changes: 14 additions & 0 deletions Framework/Nexus/test/NeXusFileTest.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,20 @@ class NeXusFileTest : public CxxTest::TestSuite {
TS_ASSERT_THROWS_NOTHING(file.makeGroup(grp, cls));
}

void test_same_make_group() {
cout << "\ntest same makeGroup\n";
FileResource resource("test_nexus_file_grp.h5");
std::string filename = resource.fullPath();
NeXus::File file(filename, NXACC_CREATE5);

string grp("test_group");

// check that we can make '/test_group/test_group'
TS_ASSERT_THROWS_NOTHING(file.makeGroup(grp, "NXsample", true));
TS_ASSERT_THROWS_NOTHING(file.makeGroup(grp, "NXdata", true));
TS_ASSERT_EQUALS(file.getPath(), "/test_group/test_group");
Copy link
Contributor

Choose a reason for hiding this comment

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

I am pretty certain, for a workspace named "data", that this line will actually create a a dataset called "/data/data/data". We need a test for this inside SaveNXSETest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes you are correct, it is opening /data/data/data. Would a nominal test with the workspace named data be sufficient?

}

void test_open_group() {
cout << "\ntest openGroup\n";
FileResource resource("test_nexus_file_grp.h5");
Expand Down
2 changes: 1 addition & 1 deletion buildconfig/CMake/CppCheck_Suppressions.txt.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveCanSAS
uselessCallsSubstr:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveFocusedXYE.cpp:73
constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveGSS.cpp:432
constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveGSS.cpp:566
constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveNXSPE.cpp:324
constVariablePointer:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveNXSPE.cpp:333
constVariableReference:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveNXTomo.cpp:150
knownConditionTrueFalse:${CMAKE_SOURCE_DIR}/Framework/DataHandling/src/SaveNexusProcessed.cpp:572
syntaxError:${CMAKE_SOURCE_DIR}/Framework/DataObjects/inc/MantidDataObjects/MortonIndex/CoordinateConversion.h:109
Expand Down