G-Node / nix

Neuroscience information exchange format
https://readthedocs.org/projects/nixio/
Other
67 stars 36 forks source link

Unexpected (incorrect) behaviour when adding multiple objects to a group #672

Closed achilleas-k closed 7 years ago

achilleas-k commented 7 years ago

Demonstration of problem

Consider the following example program:

#include <nix.hpp>

int main(){
    nix::File file = nix::File::open("append_bug.nix", nix::FileMode::Overwrite);
    nix::Block bone = file.createBlock("One", "test");
    nix::Block btwo = file.createBlock("Two", "test");
    nix::DataArray done = bone.createDataArray("DATA", "test", nix::DataType::Double, {1});
    nix::DataArray dtwo = btwo.createDataArray("DATA", "test", nix::DataType::Double, {1});  // same name

    nix::Group grp = bone.createGroup("grp", "test");

//    grp.addDataArray(dtwo);  // Uncomment to throw error

    grp.dataArrays({dtwo});  // this should throw error but succeeds

    std::cout << "dtwo name and id:     " << dtwo.name() << ", " << dtwo.id() << std::endl;
    std::cout << "appended name and id: " << grp.dataArrays()[0].name() << ", " << grp.dataArrays()[0].id() << std::endl;

    return 0;
}

The line where dtwo is added to grp via dataArrays(<vector>) succeeds even though it should fail. However, the object that gets appended isn't the one given to the method.

Output

Sample output of the program:

dtwo name and id:     DATA, 0f5a0c71-c37e-40da-83a3-8c27ca923a86
appended name and id: DATA, 7a6fe116-5dd1-4038-97ef-a573c9a43789

Description of behaviour

The dataArrays() method (https://github.com/G-Node/nix/blob/master/backend/hdf5/GroupHDF5.cpp#L108) extracts the names from the provided vector of DataArrays. It uses the names to check if the parent block contains the provided DataArrays. If it finds the DataArrays in question, it extracts their ID and passes each ID on to the addDataArray function.

The problem occurs when a DataArray passed to the dataArrays() method has the same name as a DataArray in the parent block, but is not the same object (different ID and presumably different data). The valid object gets appended, i.e., the DataArray that exists on the parent object, but this is not the DataArray that the user provided.

Solution

Once an object is obtained from the parent Block, compare the IDs as well.

jgrewe commented 7 years ago

the other solution to the problem would be to disallow adding data_arrays from different blocks... This is, what we do in MultiTags for example.

if (!block()->hasDataArray(name_or_id))
    throw std::runtime_error("MultiTagHDF5::positions: DataArray not found in block!");
achilleas-k commented 7 years ago

But that's already there. That's the rule. The problem comes from name ambiguity between DAs on different blocks.

To elaborate: the append succeeds not because dtwo is successfully linked to the group, but because the append method finds done by name.

jgrewe commented 7 years ago

solved via #674