G-Node / nix

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

Appending multiple sources simultaneously does not respect order #683

Closed achilleas-k closed 7 years ago

achilleas-k commented 7 years ago

While working on the Matlab bindings, @mpsonntag found the following issue.

When appending multiple sources to an EntityWithSources (tested with Group), they are either not appended in order, or the retrieval does not happen in order.

The following code demonstrates the issue:

#include <nix.hpp>

int main(){
    nix::File file = nix::File::open("extendorder.h5", nix::FileMode::Overwrite);
    nix::Block b = file.createBlock("blk", "blk");
    nix::Group g = b.createGroup("group", "grp");

    for (int idx = 0; idx < 10; idx++) {
        b.createSource(std::to_string(idx), "src");
    }

    g.sources(b.sources());
    for (auto src : g.sources()) {
        std::cout << src.name() << std::endl;
    }
    file.close();

    return 0;
}

(output is random).

Replacing g.sources(b.sources()) with a loop that appends sources in order works as expected however.

jgrewe commented 7 years ago

actually it works as specified :)

The order changes due to the linking mechanism. The link is done using the id, not the name. If you print out the id it is in perfect order.

achilleas-k commented 7 years ago

Interesting. Is this the intended behaviour? Should we be sorting by supplied order (as it appears in the vector)?

jgrewe commented 7 years ago

ok ok, I have to correct myself. When we create the group we specify that the oder should be by creation oder. HErr res = H5Pset_link_creation_order(gcpl.h5id(), H5P_CRT_ORDER_TRACKED | H5P_CRT_ORDER_INDEXED);

In the vector setter we order the passed entities by name, create the differences with the already present once, remove and add according to name.

The other thing I wonder: Do we really want to go long ends to maintain the order (any order)?

jgrewe commented 7 years ago

should we leave this issue open until we implement the util function for custom ordering?

gicmo commented 7 years ago

@jgrewe I'd say we open a new one for the util, and close this.

achilleas-k commented 7 years ago

Agreed