Skip to content

Commit

Permalink
Address code review feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Jackson <[email protected]>
  • Loading branch information
imikejackson committed Oct 21, 2023
1 parent 5a0d8bb commit 35f3f58
Show file tree
Hide file tree
Showing 18 changed files with 42 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ IFilter::PreflightResult AlignSectionsFeatureCentroidFilter::preflightImpl(const

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, cellDataGroupPath, {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, cellDataGroupPath, {});

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ IFilter::PreflightResult AlignSectionsListFilter::preflightImpl(const DataStruct

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, imageGeom.getCellDataPath(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, imageGeom.getCellDataPath(), {});

return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ IFilter::PreflightResult ErodeDilateBadDataFilter::preflightImpl(const DataStruc

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pFeatureIdsArrayPathValue.getParent(), pIgnoredDataArrayPathsValue);
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pFeatureIdsArrayPathValue.getParent(), pIgnoredDataArrayPathsValue);

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,9 @@ IFilter::PreflightResult MinNeighbors::preflightImpl(const DataStructure& dataSt

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, featureIdsPath.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, featureIdsPath.getParent(), {});
// Feature Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, numNeighborsPath.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, numNeighborsPath.getParent(), {});

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ IFilter::PreflightResult RemoveFlaggedFeaturesFilter::preflightImpl(const DataSt
{
// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pFeatureIdsArrayPathValue.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pFeatureIdsArrayPathValue.getParent(), {});
// Feature Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pFlaggedFeaturesArrayPathValue.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pFlaggedFeaturesArrayPathValue.getParent(), {});
}

return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ IFilter::PreflightResult RemoveMinimumSizeFeaturesFilter::preflightImpl(const Da

// Inform users that the following arrays are going to be modified in place
// Feature Data is going to be modified
complex::AppendDataModifiedActions(data, resultOutputActions.value().modifiedActions, featureGroupDataPath, {});
complex::AppendDataObjectModifications(data, resultOutputActions.value().modifiedActions, featureGroupDataPath, {});

resultOutputActions.warnings().push_back(Warning{k_NeighborListRemoval, ss});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ IFilter::PreflightResult ReplaceElementAttributesWithNeighborValuesFilter::prefl

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pComparisonDataPath.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pComparisonDataPath.getParent(), {});

return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ IFilter::PreflightResult ResampleImageGeomFilter::preflightImpl(const DataStruct

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, srcImageGeom->getCellDataPath(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, srcImageGeom->getCellDataPath(), {});
}

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ IFilter::PreflightResult AlignSectionsMisorientationFilter::preflightImpl(const

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pQuatsArrayPath.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pQuatsArrayPath.getParent(), {});

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ IFilter::PreflightResult AlignSectionsMutualInformationFilter::preflightImpl(con

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pQuatsArrayPathValue.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pQuatsArrayPathValue.getParent(), {});

return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ IFilter::PreflightResult BadDataNeighborOrientationCheckFilter::preflightImpl(co
fmt::format("The following DataArrays all must have equal number of tuples but this was not satisfied.\n{}", tupleValidityCheck.error()))};
}

resultOutputActions.value().modifiedActions.emplace_back(DataModifiedAction{pGoodVoxelsArrayPathValue, DataModifiedAction::ModifiedType::DataArrayModified});
resultOutputActions.value().modifiedActions.emplace_back(DataObjectModification{pGoodVoxelsArrayPathValue, DataObjectModification::ModifiedType::DataArrayModified});

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ IFilter::PreflightResult NeighborOrientationCorrelationFilter::preflightImpl(con

// Inform users that the following arrays are going to be modified in place
// Cell Data is going to be modified
complex::AppendDataModifiedActions(dataStructure, resultOutputActions.value().modifiedActions, pConfidenceIndexArrayPathValue.getParent(), {});
complex::AppendDataObjectModifications(dataStructure, resultOutputActions.value().modifiedActions, pConfidenceIndexArrayPathValue.getParent(), {});

return {};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ IFilter::PreflightResult RotateEulerRefFrameFilter::preflightImpl(const DataStru

std::vector<PreflightValue> preflightUpdatedValues;

resultOutputActions.value().modifiedActions.emplace_back(DataModifiedAction{pCellEulerAnglesArrayPathValue, DataModifiedAction::ModifiedType::DataArrayModified});
resultOutputActions.value().modifiedActions.emplace_back(DataObjectModification{pCellEulerAnglesArrayPathValue, DataObjectModification::ModifiedType::DataArrayModified});

// Return both the resultOutputActions and the preflightUpdatedValues via std::move()
return {std::move(resultOutputActions), std::move(preflightUpdatedValues)};
Expand Down
18 changes: 12 additions & 6 deletions src/complex/Filter/Output.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,17 +110,23 @@ class COMPLEX_EXPORT IDataCreationAction : public IDataAction
/**
* @brief
*/
struct COMPLEX_EXPORT DataModifiedAction
struct COMPLEX_EXPORT DataObjectModification
{

enum class ModifiedType : uint64
{
DataArrayModified = 0,
DataArrayPossiblyDeleted = 1,
AttributeMatrixModified = 2,
DataGroupModified = 3,
AttributeMatrixPossiblyDeleted = 4,
DataGroupPossiblyDeleted = 5,
GeometryModified = 6,
GeometryPossiblyDeleted = 7
};

DataPath m_ModifiedPath;
ModifiedType m_ModifiedType;
DataPath modifiedPath;
ModifiedType modifiedType;
};

/**
Expand All @@ -130,7 +136,7 @@ struct COMPLEX_EXPORT OutputActions
{
std::vector<AnyDataAction> actions = {};
std::vector<AnyDataAction> deferredActions = {};
std::vector<DataModifiedAction> modifiedActions = {};
std::vector<DataObjectModification> modifiedActions = {};

OutputActions() = default;

Expand All @@ -156,9 +162,9 @@ struct COMPLEX_EXPORT OutputActions
deferredActions.emplace_back(std::move(action));
}

void appendModifiedAction(const DataPath& dataPath, DataModifiedAction::ModifiedType modifiedType)
void appendDataObjectModificationNotification(const DataPath& dataPath, DataObjectModification::ModifiedType modifiedType)
{
modifiedActions.emplace_back(DataModifiedAction{dataPath, modifiedType});
modifiedActions.push_back(DataObjectModification{dataPath, modifiedType});
}

static Result<> ApplyActions(nonstd::span<const AnyDataAction> actions, DataStructure& dataStructure, IDataAction::Mode mode);
Expand Down
11 changes: 3 additions & 8 deletions src/complex/Pipeline/PipelineFilter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,9 @@ bool PipelineFilter::preflight(DataStructure& data, RenamedPaths& renamedPaths,
}
}

for(const auto& action : result.outputActions.value().modifiedActions)
{
newModifiedPaths.emplace_back(action.m_ModifiedPath);
}

// Do not clear the created paths unless the preflight succeeded
m_CreatedPaths = newCreatedPaths;
m_ModifiedPaths = newModifiedPaths;
m_DataModifiedActions = result.outputActions.value().modifiedActions;

setPreflightStructure(data);
sendFilterFaultMessage(m_Index, getFaultState());
Expand Down Expand Up @@ -227,9 +222,9 @@ std::vector<DataPath> PipelineFilter::getCreatedPaths() const
return m_CreatedPaths;
}

std::vector<DataPath> PipelineFilter::getModifiedPaths() const
std::vector<DataObjectModification> PipelineFilter::getDataObjectModificationNotifications() const
{
return m_ModifiedPaths;
return m_DataModifiedActions;
}

namespace
Expand Down
4 changes: 2 additions & 2 deletions src/complex/Pipeline/PipelineFilter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class COMPLEX_EXPORT PipelineFilter : public AbstractPipelineNode
* @brief Returns a vector of DataPaths that would be modified when executing the node
* @return std::vector<DataPath>
*/
std::vector<DataPath> getModifiedPaths() const;
std::vector<DataObjectModification> getDataObjectModificationNotifications() const;

/**
* @brief Returns a collection of warnings returned by the target filter.
Expand Down Expand Up @@ -226,6 +226,6 @@ class COMPLEX_EXPORT PipelineFilter : public AbstractPipelineNode
std::vector<complex::Error> m_Errors;
std::vector<IFilter::PreflightValue> m_PreflightValues;
std::vector<DataPath> m_CreatedPaths;
std::vector<DataPath> m_ModifiedPaths;
std::vector<DataObjectModification> m_DataModifiedActions;
};
} // namespace complex
14 changes: 8 additions & 6 deletions src/complex/Utilities/FilterUtilities.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@ Result<> CreateOutputDirectories(const fs::path& outputPath)
}

// -----------------------------------------------------------------------------
void AppendDataModifiedActions(const DataStructure& dataStructure, std::vector<DataModifiedAction>& modifiedActions, const DataPath& parentPath, const std::vector<DataPath>& ignoredDataPaths)
void AppendDataObjectModifications(const DataStructure& dataStructure, std::vector<DataObjectModification>& modifiedActions, const DataPath& parentPath, const std::vector<DataPath>& ignoredDataPaths)
{
std::optional<std::vector<DataPath>> result = complex::GetAllChildArrayDataPaths(dataStructure, parentPath, ignoredDataPaths);
if(result.has_value())
if(result->empty())
{
for(const auto& child : result.value())
{
modifiedActions.emplace_back(DataModifiedAction{child, DataModifiedAction::ModifiedType::DataArrayModified});
}
return;
}

for(const auto& child : result.value())
{
modifiedActions.push_back(DataObjectModification{child, DataObjectModification::ModifiedType::DataArrayModified});
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/complex/Utilities/FilterUtilities.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,6 @@ COMPLEX_EXPORT std::vector<char> CreateDelimitersVector(bool tabAsDelimiter, boo
* @param parentPath The parent path
* @param ignoredDataPaths And specific DataPath to ignore.
*/
COMPLEX_EXPORT void AppendDataModifiedActions(const DataStructure& dataStructure, std::vector<DataModifiedAction>& modifiedActions, const DataPath& parentPath,
const std::vector<DataPath>& ignoredDataPaths);
COMPLEX_EXPORT void AppendDataObjectModifications(const DataStructure& dataStructure, std::vector<DataObjectModification>& modifiedActions, const DataPath& parentPath,
const std::vector<DataPath>& ignoredDataPaths);
} // namespace complex

0 comments on commit 35f3f58

Please sign in to comment.