Gowerlabs / lumomat

MATLAB tools and SNIRF export for LUMO
https://www.gowerlabs.co.uk/lumo
MIT License
10 stars 2 forks source link

merge_layout fails to copy file if a string is used instead of a character vector. #23

Closed HBaghdadi1995 closed 2 years ago

HBaghdadi1995 commented 2 years ago

Setup

How to recreate

  1. Include lumomat in file.
  2. Call `lumofile.merge_layout("C:\path\to\lumo\file\lumo_file.LUMO", "C:\path\to\layout\file\layout_file.json")

What Happens

lumo_merged_fn is now generated as an array of strings, this is because [lf_name '_merged_' uid_name lf_ext] in line 148, contains a mixture of strings and character vectors. Compare fullfile('foobar',['foo' 'bar']) with fullfile('foobar',["foo" 'bar']) as an example.

Conclusion

This issue should be an easy fix, but there are multiple solutions around it.

samuelpowell commented 2 years ago

I do not see this in local testing. I assume this is because you called a function using a string rather than a character vector. Please can you provide an example to reproduce?

samuelpowell commented 2 years ago

I do not see this in local testing. I assume this is because you called a function using a string rather than a character vector. Please can you provide an example to reproduce?

HBaghdadi1995 commented 2 years ago

I assume this is because you called a function using a string rather than a character vector

That was it, my bad.

It happened when I called lumofile.merge_layout("C:\Users\Hasan\Downloads\Lumo_samples_M3\0.4.0.LUMO", "C:\Users\Hasan\AppData\Local\Gowerlabs\LUMO\coordinates_xxxxxxxx.json").

And it worked when I called lumofile.merge_layout('C:\Users\Hasan\Downloads\Lumo_samples_M3\0.4.0.LUMO', 'C:\Users\Hasan\AppData\Local\Gowerlabs\LUMO\coordinates_xxxxxxxx.json')

HBaghdadi1995 commented 2 years ago

I've rewritten the issue and the comment, I'll have to check #24 incase it breaks character arrays.

It's ultimatley up to you to decide if we should accept both strings and character arrays, or only character arrays

samuelpowell commented 2 years ago

It will break character arrays. The correct thing is to do some input validation and ensure that things are all character arrays at the user boundary.

HBaghdadi1995 commented 2 years ago

Okay sure, I'll close this issue and the pr then.