bastibe / transplant

Transplant is an easy way of calling Matlab from Python
https://transplant.readthedocs.io
Other
110 stars 26 forks source link

MATLAB:Containers:Map doesn't allow empty keys #32

Closed Foucl closed 7 years ago

Foucl commented 7 years ago

While trying to work with Labstreaming Layer's xdf-files and the way they are read into matlab by their importer, I ran into an issue in dumpmsgpack.m. LSL's reader function generates deeply nested structs that sometimes have empty 'substructs'.

Example: In Matlab:

function [ s ] = gen_struct_empty_fields(  )
    s = struct();
    s.field1 = 'test';
    s.field2 = struct();
end

Trying to execute this from a transplant.Matlab() instance (s = matlab.gen_struct_empty_fields()) throws the following traceback:

Traceback (most recent call last):

  File "<ipython-input-13-55a6fd42c0fa>", line 1, in <module>
    matlab.gen_struct_empty_fields()

  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_master.py", line 481, in __call__
    return self._call(data[1], args, nargout=nargout)

  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_master.py", line 461, in _call
    nargout=nargout)

  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_master.py", line 163, in send_message
    response['stack'], response['identifier'], response['message'])

TransplantError: Empty keys are not allowed in this container. (MATLAB:Containers:Map:EmptyKey)
Traceback (most recent call last):
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_remote.m", line 128, in transplant_remote
    send_value(results{1});
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_remote.m", line 187, in transplant_remote/send_value
    send_message('value', message);
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\transplant_remote.m", line 155, in transplant_remote/send_message
    messenger.send(dumpmsgpack(message));
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 22, in dumpmsgpack
    msgpack = dump(data);
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 68, in dump
    msgpack = dumpmap(data);
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 219, in dumpmap
    valuestuff = dump(values{n});
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 68, in dump
    msgpack = dumpmap(data);
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 219, in dumpmap
    valuestuff = dump(values{n});
  File "C:\Users\chdan\Anaconda2\envs\mymu\lib\site-packages\transplant\dumpmsgpack.m", line 46, in dump
    data = containers.Map(fieldnames(data), struct2cell(data));

For me, simply doing this

if isstruct(data)
    if isempty(fieldnames(data))
        data = [];
    else
        data = containers.Map(fieldnames(data), struct2cell(data));
    end
end

in dumpmsgpack.m L45 solves the issue (at least for my use case). Would this be worthy of a pull request?

bastibe commented 7 years ago

Thank you for finding another bug in Transplant!

Would this be worthy of a pull request?

Absolutely! Although I would prefer

if isstruct(data)
    if ~isempty(fieldnames(data))
        data = containers.Map(fieldnames(data), struct2cell(data));
    else
        data = containers.Map();
    end
end

Personally, I'd prefer it if you did a pull request instead of me doing it, just to document your contribution in the commit history. If that's too much work, I can do the commit as well.