G-Node / nix

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

Removing dimensions #593

Closed jgrewe closed 8 years ago

jgrewe commented 8 years ago

So far, we treat dimensions as a stack. Let's assume we have 2 dimensions in one DataArray. If the first one is removed the second one becomes the new number 1. That means, the group is renamed and the index is updated to 1. This is kind-of-strange behavior, especially since we do not support inserting of dimensions.

As I see it, we have two options. 1) We support inserting dimensions, this requires to replace the DataArray::appendDimension methods. 2) Allow setting and removing dimensions only in unison

There is one more issue:

    double samplingInterval = boost::math::constants::pi<double>();
    Dimension d = data_array.appendSampledDimension(samplingInterval);
    Dimension d2 = data_array.appendSampledDimension(samplingInterval);

    data_array.deleteDimension(d.index());
    // at this point group and index of d2 in the backend should have changed.
    std::cerr << d2.index() << std::endl;
    // actually, the output will be 2 though the current index of d2 should be 1
    // the following command works with the hdf5 backend, but fails, correctly, in the fs backend.
    data_array.deleteDimension(d2.index());
sudoankit commented 8 years ago

I want to work on this. I am reading and checking the fs backend. Is it the right way? Please guide me how to start fixing this.

jgrewe commented 8 years ago

This issue relates to both backends, not only the file-system. The tricky part is to find out which way to go. I personally think, that we should not handle the dimensions as a stack. Rather the dimensions should keep their index and should not be renamed. This would need mechanism for the insertion and the deletion of selected dimensions.

jgrewe commented 8 years ago

@sudoankit if you want to tackle this issue, this would be the way to go.

  1. disallow the removal of individual dimensions, i.e remove the respective methods.
  2. provide a method to remove all dimensions at once.
  3. do not re-arrange remaining dimensions.
  4. adapt test cases.
  5. check both back-ends.

Please check out the Contributing.md file. If you need more info, let us know.

sudoankit commented 8 years ago

Thank you so much. Danke schön. I am understanding the classes and methods used in NIX and brushing up the boost library. I am learning to write test cases. It's going well. When in doubt can I ask you for help in the IRC chat or mail or here at Github? Do we have a gitter chat room for G-Node/NIX? I have read the contributing file. Also, can I be assigned to this?

First, I will try to recreate and understand methods for dimensions. I will update you with my work regularly.

[out of context: the contributing file has a small typo in 'issue tracker section'. crated -> created]

jgrewe commented 8 years ago

@sudoankit, you're welcome! I have seen you popping up in our IRC channel, sure, you can post your message there.

sudoankit commented 8 years ago

I had a question. Won't the index start from 0? And thus, why add +1 to getDimension ?

std::vector<Dimension> DataArray::dimensions(const util::Filter<Dimension>::type &filter) const {
    auto f = [this] (size_t i) { return getDimension(i+1); }; // +1 since index starts at 1
    return getEntities<Dimension>(f,
                                  dimensionCount(),
                                  filter);
}