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

updated getRow function to slice through last dimension of multidimensional columns #378

Merged
merged 6 commits into from
Jan 7, 2022

Conversation

cechava
Copy link
Collaborator

@cechava cechava commented Jan 4, 2022

fixes #377

…sional matrix; works with simple DynamicTable
@cechava
Copy link
Collaborator Author

cechava commented Jan 4, 2022

with initial commit, getRow method works with simple DynamicTable objects

sample snippet

col1 = types.hdmf_common.VectorData( ...
    'description', 'column #1', ...
    'data', (1:10)' ...
);
col2 = types.hdmf_common.VectorData( ...
    'description', 'column #2', ...
    'data', rand(5,2,10) ...
);
my_table = types.hdmf_common.DynamicTable( ...
    'description','an example table', ...
    'colnames', {'col1','col2'}, ...
    'col1',col1, ...
    'col2',col2, ...
    'id', types.hdmf_common.ElementIdentifiers('data', (0:9)') ...
);
>> my_table.getRow(1:3)

ans =

  3×2 table

    col1         col2     
    ____    ______________

     1      [1×5×2 double]
     2      [1×5×2 double]
     3      [1×5×2 double]

@bendichter
Copy link
Contributor

Could we make the rows 5x2 instead of 1x5x2?

@cechava
Copy link
Collaborator Author

cechava commented Jan 4, 2022

Could we make the rows 5x2 instead of 1x5x2?

it's not immediately clear how to do that. That output is handled under the hood by MATLAB after running the following line

subTable = table(row{:}, 'VariableNames', columns);

with the following variable values

K>> row

row =

  1×2 cell array

    {3×1 double}    {3×5×2 double} 
    
K>> columns

columns =

  1×2 cell array

    {'col1'}    {'col2'} 

@codecov
Copy link

codecov bot commented Jan 5, 2022

Codecov Report

Merging #378 (f3a71e8) into master (d855b78) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
+ Coverage   84.91%   85.06%   +0.14%     
==========================================
  Files         124      124              
  Lines        4495     4539      +44     
==========================================
+ Hits         3817     3861      +44     
  Misses        678      678              
Impacted Files Coverage Δ
+tests/+system/DynamicTableTest.m 88.14% <100.00%> (+2.16%) ⬆️
+types/+util/+dynamictable/getRow.m 97.64% <100.00%> (+0.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d855b78...f3a71e8. Read the comment docs.

@cechava cechava marked this pull request as ready for review January 5, 2022 23:58
@cechava cechava changed the title updated getRow function to slide through last dimension of multidimensional columns updated getRow function to slice through last dimension of multidimensional columns Jan 5, 2022
@cechava cechava requested a review from bendichter January 6, 2022 17:18
@cechava cechava self-assigned this Jan 7, 2022
@cechava
Copy link
Collaborator Author

cechava commented Jan 7, 2022

@ln-vidrio @bendichter

Let me know if you have any thoughts on this PR.
I modified the DynamicTable tests to not use addRow because that hasn't been updated and generates tables with the wrong dimensions. We can easily add that back in once it has been updated.

@cechava cechava merged commit 0858a9f into NeurodataWithoutBorders:master Jan 7, 2022
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.

getRow slices wrong dimension for multidimensional matrices
3 participants