Skip to content

BUG: C05: Child DataPath built by substring replace in 5 filters #1865

Open

Description

@imikejackson

Summary

Some filters build the DataPath of a child object with string operations or from the root, not from the parent DataPath. Examples are DataPath({name}), DataPath({parent.getTargetName(), ...}), sibling.replaceName(...) on a path in a different Attribute Matrix, and StringUtilities::replace on DataPath::toString(). The result is correct only when the parent object is at the root and no other name in the path contains the parent's name. Otherwise the filter writes to the wrong place or reads a path that does not exist.

As of develop 419f9db (2026-10-21) this class has 5 open findings in 5 filters: 2 CRASH, 3 WRONG_DATA, 0 MINOR. All findings are candidates. They were found and re-checked by reading the code, not by running it.

Mechanism

A DataPath is a list of object names. Preflight usually creates the output object at the correct path, because it uses the selected DataPath. Execute or a second preflight step then builds the same path again in a different way. There are three forms:

  • Path built from the root. DataPath({geom.getTargetName(), am, name}) or DataPath({name + "_resampled"}) drops every parent of the geometry. For a geometry in a DataGroup, the path does not exist (getDataAs returns nullptr and the code dereferences it), or the object is created at the root.
  • Path built from an unrelated sibling. QuatsArrayPath.replaceName(featureIdsName) assumes that the output array is in the same Attribute Matrix as an input array. Preflight created it somewhere else, so execute gets nullptr.
  • Substring replace on the path string. StringUtilities::replace(childPath.toString(), srcName, destName) replaces every occurrence of the name, not only the path component of the geometry. Grain/Grain Data is changed in two places.

The shared code in GeometryTransformation.cpp still uses the third form on develop. ApplyTransformationToGeometryFilter and ReadMhaFileFilter both call it:

src/simplnx/Utilities/Geometry/GeometryTransformation.cpp#L304-L307

        for(const auto& childPath : childPaths.value())
        {
          const std::string copiedChildName = nx::core::StringUtilities::replace(childPath.toString(), srcImagePath.getTargetName(), destImagePath.getTargetName());
          const DataPath copiedChildPath = DataPath::FromString(copiedChildName).value();

PR #1765 (1986127) fixed seven filters with the same loop. They now use DataPath::rebase, which replaces only the path prefix:

src/Plugins/SimplnxCore/src/SimplnxCore/Filters/PadImageGeometryFilter.cpp#L252-L254

    for(const auto& childPath : childPaths.value())
    {
      const DataPath copiedChildPath = childPath.rebase(srcImagePath, destImagePath).value();

Rule

Build every DataPath from the DataPath that the user selected or that preflight created. Do not build a DataPath from a name string or with string operations.

This rule follows the fixes in PR #1765. The project lead has not written it down as a class rule; treat it as the proposed rule.

  • Use parentPath.createChildPath(name) for a new child.
  • Use path.replaceName(newName) only for a sibling in the same parent.
  • Use childPath.rebase(oldPrefix, newPrefix) to move a path from one geometry to another.
  • Execute must use the same path that preflight created. Compute it once in the same way in both places, or compute it in one helper.

How to fix

  1. Find every DataPath({...getTargetName()...}), DataPath({name + ...}) and StringUtilities::replace(...toString()...) in the filter and its algorithm.
  2. Replace them with createChildPath, replaceName or rebase (all exist in src/simplnx/DataStructure/DataPath.hpp at 419f9db; rebase returns std::optional<DataPath>).
  3. In GeometryTransformation.cpp, change lines 306 and 316 to childPath.rebase(srcImagePath, destImagePath).value() and sourcePath.rebase(srcImagePath, destImagePath).value(), as in PadImageGeometryFilter.cpp. This one change fixes both ApplyTransformationToGeometryFilter and ReadMhaFileFilter.
  4. In ReadImageStackFilter, use imageGeomPath.replaceName(imageGeomPath.getTargetName() + "_resampled") at lines 297 and 343, and the same expression in ReadImageStack.cpp:333.
  5. In EBSDSegmentFeaturesFilter, build FeatureIdsArrayPath in execute from the geometry's getCellDataPath(), as preflight does at lines 129-130. Do not require the input Quaternions or Phases to be in that Attribute Matrix: a filter that only reads an element DataArray needs only a matching tuple count (see the tuple-count rule from BUG: Require Feature Ids in the geometry's Cell Attribute Matrix for filters that modify it #1829 and PR BUG: Check that grid-geometry cell inputs have one tuple per cell #1840).
  6. In CombineStlFilesFilter, use TriangleDataContainerName.createChildPath(vertexAM).createChildPath(name), which is the path preflight creates at line 184.
  7. If a fix needs a new error code, use the filter's existing error-code range. Check that the code is unique with git grep -nE -- "-NNNN([^0-9]|$)" on develop and on the open PR branches.

How to test

  • Write the test first. It must fail on develop.
  • Put the parent object in a DataGroup (for example Group/ImageGeom), not at the root. Run preflight and execute.
  • For a CRASH finding, the test must pass execute without a crash and the output must exist at the expected path.
  • For a WRONG_DATA finding, check that every output object is at the expected DataPath and that no object was created at the root.
  • For the substring case, use names that contain the geometry name: geometry Grain with Attribute Matrix Grain Data, or geometry Image in DataGroup Image Group. Check the full DataPath of each copied child.
  • Check output values only against an independent expected result (oracle), not against the output of the code under test.

Conventions

  • One PR for the whole class, one commit per filter. Commit prefix BUG: with a Signed-off-by line. No AI attribution.
  • Every fix starts with a unit test that fails on develop. If you cannot reproduce a finding, write that and the reason in the PR and on this issue. Do not force a fix.
  • Value checks need an independent expected result (oracle), not the code's own output.
  • Run ctest serially. Never use -j; the tests depend on each other.
  • Format with clang-format 19.1.1 and the repository .clang-format.
  • Do not add sentences about input requirements to the filter documentation. The generated parameter table documents them.
  • If a fix touches a filter that has a V&V report (src/Plugins/<Plugin>/vv/<Filter>.md), update every file of its V&V package (report, deviations, provenance, test tables). V&V documents reference DREAM3D 6.5.171 only.
  • Rebase on develop. Never merge develop into the branch.

Checklist

  • CRASH · CombineStlFilesFilter · CombineStlFilesFilter.cpp:231 — Execute builds the vertex label path from the root with getTargetName(). For a nested Triangle Geometry (Group/TriGeom) the path does not exist and execute dereferences nullptr. CombineStlFilesFilter#1
  • CRASH · EBSDSegmentFeaturesFilter · EBSDSegmentFeaturesFilter.cpp:184 — Preflight creates Feature Ids in the geometry's Cell Data, but execute uses QuatsArrayPath.replaceName(...). With Cell Quaternions in another Attribute Matrix, execute calls fill(0) on nullptr. EBSDSegmentFeaturesFilter#1
  • WRONG_DATA · ApplyTransformationToGeometryFilter · GeometryTransformation.cpp:306 — Child paths are made with StringUtilities::replace on the full path string (lines 306, 316). Other names that contain the geometry name (Grain Data in Grain) are also renamed. ApplyTransformationToGeometryFilter#7
  • WRONG_DATA · ReadImageStackFilter · ReadImageStackFilter.cpp:297 — With Resample Images on and a nested Created Image Geometry, the temporary path is DataPath({name + "_resampled"}) at the root (lines 297, 343, ReadImageStack.cpp:333), so the output is not in its group. ReadImageStackFilter#3
  • WRONG_DATA · ReadMhaFileFilter · GeometryTransformation.cpp:306 — With Apply Image Transformation To Geometry on, the shared GeometryTransformation.cpp code renames child names that contain the geometry name (for example Image/ImageMatrix). ReadMhaFileFilter#1

Already fixed or in progress

Fixed on develop by #1765 (1986127), which moved these filters to DataPath::rebase:

  • CopyDataObjectFilter CopyDataObjectFilter#2
  • CropEdgeGeometryFilter CropEdgeGeometryFilter#2
  • CropImageGeometryFilter CropImageGeometryFilter#1
  • PadImageGeometryFilter PadImageGeometryFilter#3
  • RemoveFlaggedVerticesFilter RemoveFlaggedVerticesFilter#2
  • ResampleImageGeomFilter ResampleImageGeomFilter#3
  • RotateSampleRefFrameFilter RotateSampleRefFrameFilter#1

Out of scope

  • No finding of this class is in a filter that has a V&V report (0 excluded).
  • Other DataPath problems (array kind, tuple count, Attribute Matrix requirements) belong to their own issues.

Activity

  1. changed the title [-]BUG: Child DataPath built by substring replace in 5 filters[/-] [+]BUG: C05: Child DataPath built by substring replace in 5 filters[/+] on Oct 22, 2026
  2. self-assigned this
    on Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions