Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dataAccess] bind position and count dimension to the array extent ... #755

Merged
merged 1 commit into from
Oct 29, 2018

Conversation

jgrewe
Copy link
Member

@jgrewe jgrewe commented Oct 29, 2018

this solves an issue in which a {1,1} dataset could not be read

this solves an issue in which a {1,1} dataset could not be read
@achilleas-k
Copy link
Member

Just to clarify, the old version would use the number of dimensions attached to an array to calculate offset and count, which would cause issues with {1, 1} arrays that had only one dimension object attached? And with this change, it now uses the actual shape of the data instead.

@jgrewe
Copy link
Member Author

jgrewe commented Oct 29, 2018

yes, this is true, but I agree the message was not good. Let me try again:

In cases that the data is 1D there is only a single dimension. When multi-tagging such data we require the tag positions array to be 2d (1st dim the number of points, second dim the number of dimension in the referenced data, 1d in this example). When reading the first position we must now read a slice form the positions array that is {1, 1} which is not equal to {1}! (This was the case in the previous version and failed)

edit:
reading the position now depends on the shape of the positions array and no longer the referenced DataArray( does that make sense?)

@achilleas-k
Copy link
Member

yes, this is true, but I agree the message was not good. Let me try again:

Nah, the change was pretty clear from the message and the code change. Just wanted to make sure I was understanding the underlying issue correctly.

@achilleas-k achilleas-k merged commit 46b6c51 into G-Node:1.4 Oct 29, 2018
@jgrewe jgrewe deleted the data_access branch October 29, 2018 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants