Skip to content

Commit 9aa1373

Browse files
nyoungbqimikejackson
nyoungbq
authored and
imikejackson
committed
ENH: Add Additional Warnings When Moving/Copying DataObjects with a Tuple Mismatch (BlueQuartzSoftware#804)
1 parent c4a5e1a commit 9aa1373

File tree

5 files changed

+124
-13
lines changed

5 files changed

+124
-13
lines changed

‎src/Plugins/SimplnxCore/src/SimplnxCore/Filters/CopyDataObjectFilter.cpp

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "CopyDataObjectFilter.hpp"
22

3+
#include "simplnx/DataStructure/AttributeMatrix.hpp"
34
#include "simplnx/DataStructure/BaseGroup.hpp"
45
#include "simplnx/Filter/Actions/CopyDataObjectAction.hpp"
56
#include "simplnx/Parameters/BoolParameter.hpp"
@@ -80,14 +81,35 @@ IFilter::PreflightResult CopyDataObjectFilter::preflightImpl(const DataStructure
8081
return {MakeErrorResult<OutputActions>(-27360, "Copied Object(s) have the same parent as original data but the suffix is empty."), {}};
8182
}
8283

83-
OutputActions actions;
84+
Result<OutputActions> resultOutputActions;
8485

8586
for(const auto& dataArrayPath : dataArrayPaths)
8687
{
8788
DataPath parentPath = dataArrayPath.getParent();
8889
if(useNewParent)
8990
{
9091
parentPath = args.value<DataPath>(k_NewPath_Key);
92+
// Scope AM check since we fully expect it to be a nullptr
93+
{
94+
const auto* possibleAM = data.getDataAs<AttributeMatrix>(parentPath);
95+
if(possibleAM != nullptr)
96+
{
97+
for(const auto& path : dataArrayPaths)
98+
{
99+
const auto* possibleIArray = data.getDataAs<IArray>(path);
100+
if(possibleIArray != nullptr)
101+
{
102+
if(possibleAM->getShape() != possibleIArray->getTupleShape())
103+
{
104+
resultOutputActions.warnings().push_back(
105+
Warning{-27361, fmt::format("The tuple dimensions of {} [{}] do not match the AttributeMatrix {} tuple dimensions [{}]. This could result in a runtime error if the sizing remains "
106+
"the same at time of this filters execution. Proceed with caution.",
107+
possibleIArray->getName(), possibleIArray->getNumberOfTuples(), possibleAM->getName(), possibleAM->getNumTuples())});
108+
}
109+
}
110+
}
111+
}
112+
}
91113
}
92114
std::string newTargetName = dataArrayPath.getTargetName() + suffix;
93115
DataPath newDataPath = parentPath.createChildPath(newTargetName);
@@ -100,16 +122,16 @@ IFilter::PreflightResult CopyDataObjectFilter::preflightImpl(const DataStructure
100122
{
101123
for(const auto& sourcePath : pathsToBeCopied.value())
102124
{
103-
std::string createdPathName = nx::core::StringUtilities::replace(sourcePath.toString(), dataArrayPath.getTargetName(), newTargetName);
125+
std::string createdPathName = StringUtilities::replace(sourcePath.toString(), dataArrayPath.getTargetName(), newTargetName);
104126
allCreatedPaths.push_back(DataPath::FromString(createdPathName).value());
105127
}
106128
}
107129
}
108130
auto action = std::make_unique<CopyDataObjectAction>(dataArrayPath, newDataPath, allCreatedPaths);
109-
actions.appendAction(std::move(action));
131+
resultOutputActions.value().appendAction(std::move(action));
110132
}
111133

112-
return {std::move(actions)};
134+
return {std::move(resultOutputActions)};
113135
}
114136

115137
//------------------------------------------------------------------------------

‎src/Plugins/SimplnxCore/src/SimplnxCore/Filters/MoveData.cpp

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "MoveData.hpp"
22

3+
#include "simplnx/DataStructure/AttributeMatrix.hpp"
34
#include "simplnx/DataStructure/BaseGroup.hpp"
45
#include "simplnx/Filter/Actions/MoveDataAction.hpp"
56
#include "simplnx/Parameters/DataGroupSelectionParameter.hpp"
@@ -10,13 +11,6 @@
1011

1112
namespace nx::core
1213
{
13-
namespace
14-
{
15-
constexpr int64 k_TupleCountInvalidError = -250;
16-
constexpr int64 k_MissingFeaturePhasesError = -251;
17-
18-
} // namespace
19-
2014
//------------------------------------------------------------------------------
2115
std::string MoveData::name() const
2216
{
@@ -74,6 +68,27 @@ IFilter::PreflightResult MoveData::preflightImpl(const DataStructure& data, cons
7468
Result<OutputActions> resultOutputActions;
7569
std::vector<PreflightValue> preflightUpdatedValues;
7670

71+
// Scope AM check since we fully expect it to be a nullptr
72+
{
73+
const auto* possibleAM = data.getDataAs<AttributeMatrix>(newParentPath);
74+
if(possibleAM != nullptr)
75+
{
76+
for(const auto& path : dataPaths)
77+
{
78+
const auto* possibleIArray = data.getDataAs<IArray>(path);
79+
if(possibleIArray != nullptr)
80+
{
81+
if(possibleAM->getShape() != possibleIArray->getTupleShape())
82+
{
83+
resultOutputActions.warnings().push_back(Warning{-69250, fmt::format("The tuple dimensions of {} [{}] do not match the AttributeMatrix {} tuple dimensions [{}]. This could result in a "
84+
"runtime error if the sizing remains the same at time of this filters execution. Proceed with caution.",
85+
possibleIArray->getName(), possibleIArray->getNumberOfTuples(), possibleAM->getName(), possibleAM->getNumTuples())});
86+
}
87+
}
88+
}
89+
}
90+
}
91+
7792
for(const auto& dataPath : dataPaths)
7893
{
7994
auto moveDataAction = std::make_unique<MoveDataAction>(dataPath, newParentPath);

‎src/Plugins/SimplnxCore/test/CopyDataObjectTest.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88

99
using namespace nx::core;
1010

11+
namespace
12+
{
13+
const int32 k_TupleDimMismatchWarningCode = -27361;
14+
}
15+
1116
TEST_CASE("SimplnxCore::CopyDataObjectFilter(Valid Execution)", "[SimplnxCore][CopyDataObjectFilter]")
1217
{
1318
static const DataPath k_DataPath1({Constants::k_SmallIN100, "Phase Data"});
@@ -95,6 +100,22 @@ TEST_CASE("SimplnxCore::CopyDataObjectFilter(Invalid Parameters)", "[SimplnxCore
95100
args.insert(CopyDataObjectFilter::k_NewPath_Key, std::make_any<DataPath>(copyPath));
96101
args.insert(CopyDataObjectFilter::k_NewPathSuffix_Key, std::make_any<std::string>("_COPY"));
97102

103+
// Preflight the filter and check result
104+
auto preflightResult = filter.preflight(dataStructure, args);
105+
106+
REQUIRE_FALSE(preflightResult.outputActions.warnings().empty());
107+
108+
bool found = false;
109+
for(const auto& warning : preflightResult.outputActions.warnings())
110+
{
111+
if(warning.code == ::k_TupleDimMismatchWarningCode)
112+
{
113+
found = true;
114+
}
115+
}
116+
117+
REQUIRE(found);
118+
98119
auto result = filter.execute(dataStructure, args);
99120
SIMPLNX_RESULT_REQUIRE_INVALID(result.result);
100121
}

‎src/Plugins/SimplnxCore/test/MoveDataTest.cpp

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "SimplnxCore/Filters/MoveData.hpp"
22

3+
#include "simplnx/DataStructure/AttributeMatrix.hpp"
4+
#include "simplnx/DataStructure/DataArray.hpp"
35
#include "simplnx/DataStructure/DataGroup.hpp"
46
#include "simplnx/UnitTest/UnitTestCommon.hpp"
57

@@ -16,6 +18,11 @@ constexpr StringLiteral k_Group2Name = "Group2";
1618
constexpr StringLiteral k_Group3Name = "Group3";
1719
constexpr StringLiteral k_Group4Name = "Group4";
1820

21+
const int32 k_TupleDimMismatchWarningCode = -69250;
22+
23+
constexpr StringLiteral k_AM1Name = "Matrix1";
24+
constexpr StringLiteral k_DataArray1Name = "Array1";
25+
1926
DataStructure createDataStructure()
2027
{
2128
DataStructure data;
@@ -24,6 +31,11 @@ DataStructure createDataStructure()
2431
auto group3 = DataGroup::Create(data, k_Group3Name, group2->getId());
2532
auto group4 = DataGroup::Create(data, k_Group4Name, group2->getId());
2633

34+
auto am1 = AttributeMatrix::Create(data, k_AM1Name, std::vector<usize>{2});
35+
36+
auto dataStore = std::make_unique<DataStore<uint8>>(std::vector<usize>{20}, std::vector<usize>{3}, 0);
37+
auto dataArray1 = DataArray<uint8>::Create(data, k_DataArray1Name, std::move(dataStore));
38+
2739
data.setAdditionalParent(group4->getId(), group3->getId());
2840
return data;
2941
}
@@ -80,19 +92,61 @@ TEST_CASE("SimplnxCore::MoveData Unsuccessful", "[Complex::Core][MoveData]")
8092
{
8193
args.insertOrAssign(MoveData::k_Data_Key, std::make_any<std::vector<DataPath>>({k_Group3Path}));
8294
args.insertOrAssign(MoveData::k_NewParent_Key, std::make_any<DataPath>(k_Group2Path));
95+
96+
// Preflight the filter and check result
97+
auto preflightResult = filter.preflight(dataStructure, args);
98+
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
8399
}
84100
SECTION("Cannot reparent object to itself")
85101
{
86102
args.insertOrAssign(MoveData::k_Data_Key, std::make_any<std::vector<DataPath>>({k_Group3Path}));
87103
args.insertOrAssign(MoveData::k_NewParent_Key, std::make_any<DataPath>(k_Group3Path));
104+
105+
// Preflight the filter and check result
106+
auto preflightResult = filter.preflight(dataStructure, args);
107+
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
88108
}
89109
SECTION("Cannot reparent object to a child object")
90110
{
91111
args.insertOrAssign(MoveData::k_Data_Key, std::make_any<std::vector<DataPath>>({k_Group2Path}));
92112
args.insertOrAssign(MoveData::k_NewParent_Key, std::make_any<DataPath>(k_Group3Path));
113+
114+
// Preflight the filter and check result
115+
auto preflightResult = filter.preflight(dataStructure, args);
116+
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
93117
}
118+
}
119+
120+
TEST_CASE("SimplnxCore::MoveData Tuple Size Mismatches Warning and Failure", "[Complex::Core][MoveData]")
121+
{
122+
MoveData filter;
123+
Arguments args;
124+
DataStructure dataStructure = createDataStructure();
125+
126+
const DataPath k_DataArrayPath({k_DataArray1Name});
127+
const DataPath k_AMPath({k_AM1Name});
128+
129+
args.insertOrAssign(MoveData::k_Data_Key, std::make_any<std::vector<DataPath>>({k_DataArrayPath}));
130+
args.insertOrAssign(MoveData::k_NewParent_Key, std::make_any<DataPath>(k_AMPath));
94131

95132
// Preflight the filter and check result
96133
auto preflightResult = filter.preflight(dataStructure, args);
97-
SIMPLNX_RESULT_REQUIRE_INVALID(preflightResult.outputActions);
134+
135+
REQUIRE_FALSE(preflightResult.outputActions.warnings().empty());
136+
137+
bool found = false;
138+
for(const auto& warning : preflightResult.outputActions.warnings())
139+
{
140+
if(warning.code == ::k_TupleDimMismatchWarningCode)
141+
{
142+
found = true;
143+
}
144+
}
145+
146+
REQUIRE(found);
147+
148+
SIMPLNX_RESULT_REQUIRE_VALID(preflightResult.outputActions);
149+
150+
auto result = filter.execute(dataStructure, args);
151+
SIMPLNX_RESULT_REQUIRE_INVALID(result.result);
98152
}

‎src/simplnx/Filter/Actions/MoveDataAction.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include "MoveDataAction.hpp"
22

33
#include "simplnx/DataStructure/BaseGroup.hpp"
4-
#include "simplnx/Utilities/DataArrayUtilities.hpp"
54

65
#include <fmt/core.h>
76

0 commit comments

Comments
 (0)