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

fix edge case in marker value import #768

Merged
merged 6 commits into from
Sep 4, 2024
Merged

Conversation

dominikbach
Copy link
Contributor

@dominikbach dominikbach commented Aug 27, 2024

Fixes #727, an edge case reported in #719.

Changes proposed in this pull request:

  • generalise syntax for [S(:).field] for cases in which numel(S(i).field) ~= 1 in a new function pspm_struct2vec
  • call this in several import functions
  • fix a bug that leads to markerinfos.values being char rather than num.

@dominikbach dominikbach marked this pull request as draft August 27, 2024 07:42
@dominikbach dominikbach requested a review from teddychao August 27, 2024 13:08
@dominikbach dominikbach self-assigned this Aug 27, 2024
@dominikbach dominikbach added Bug Something isn't working Completed & Waiting for Review Completed and waiting for review labels Aug 27, 2024
@dominikbach dominikbach added this to the v7.0 milestone Aug 27, 2024
@dominikbach dominikbach marked this pull request as ready for review August 27, 2024 13:09
end

% for loops are faster than arrayfun
n = numel(S);
Copy link
Contributor

@teddychao teddychao Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this function with two-dimensional structure array, and I can see it works well with this function. The only thing is, this function will provide a 1xN vector v, where N denotes the number of elements in the structure array. If it is a two dimensional structure array with size p x q, the output will be 1 x p*q vector. If this is expected, I can merge this PR directly. Thank you.

Copy link
Contributor

@teddychao teddychao Sep 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing code:

field = 'f';
value = {'some text','a';
[10, 20, 30],'b';
magic(5),3};
S = struct(field,value);
v = pspm_struct2vec(S, 'f')

@teddychao teddychao added Approved The pull request has been approved and can be checked and then merged. and removed Bug Something isn't working Completed & Waiting for Review Completed and waiting for review labels Sep 4, 2024
@dominikbach dominikbach merged commit e0848bc into develop Sep 4, 2024
1 check passed
@dominikbach dominikbach deleted the fix-marker-values branch September 4, 2024 05:05
@teddychao teddychao removed the Approved The pull request has been approved and can be checked and then merged. label Sep 9, 2024
@teddychao teddychao mentioned this pull request Oct 12, 2024
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.

Check bdf import
2 participants