Tue 29 Sep 2015 06:54:51 PM UTC, comment #8:
Thanks for the elegant patch, Rik!
Seems that I was a couple of hours too late with my testing, this time. But I HAVE tested your patch, and observed that it fully fixes the topic of this bug. And I couldn't find any harmful sideeffects of this patch either. Now it's pushed to the repository, anyways. So far so good.
But I also stumbled across the three "Fixme, is it Matlab compatible?" comments in the test section of text.m . And I did run those three tests on Matlab, to check for Matlab compatibility. Doing this, I observed several incompatiblities which I will list in the remainder of this post. Maybe you, Rik, could give me some feedback if it's worth posting a seperate bug report about any of those.
But first the details. I run the following script both in Matlab 2013b and in 2014b:
The result in Matlab 2013b was:
The result in Matlab 2014b was:
The result with Octave 4.0.0 with your latest patch (of this bug report) applied is:
From this I have drawn the following conclusions:
- The second "fixme test" ("Multiple objects with same multi-line string which has empty cell") isn't Matlab compatible at all. I needed to add two numbers to the code, in order to make it work: h = text ([0.7, 0.7, 0.7], [0.3, 0.5, 0.7], {'Line1'; []; 'Line3'}); This means: If you give Matlab a cell array with three entries and only two x-y-coordinates (as does the original test), it will throw an error ("Each string specified must have a corresponding set of coordinates"). Octave currently does NOT throw this error. Instead Octave will plot each text line twice! So Octave is more tolerant, but produces unexpected results. Shall I open a seperate bug report for this? (A)
- The first "fixme test" produces cell results with Octave. This is the same result as Matlab 2014b. In Matlab 2013b the result was still a char. So I would consider this (currently) Matlab compatible -> OK.
- The second "fixme test" (corrected, as discussed above) produces cell results with Octave. But both in Matlab 2013b and 2014b it produces character arrays as results. So this is NOT Matlab compatible right now. Shall I open a seperate bug report for this? (B)
- The third "fixme test" produces character array as result, in Octave as well as in Matlab 2013b and 2014b. So this test is fully Matlab compatible right now. This means you could remove the comment "## FIXME: is return value really char in Matlab?" in the current source code of this Octave test. (C)
Let me know if you think it's useful to open a new bug report for any of the two real incompatiblities (A or B) listed above. If you like to, go ahead and just remove the fixme comment in the third test (C).
|
Sat 26 Sep 2015 07:14:49 PM UTC, comment #5:
Ooops, yes. Very simple "text" commands like this don't work any more with this first patch:
close all, text(0.5, 0.5, 'Hello')
It seemFs to me that you try to index a cell array (with curly brackets) in your patch. This one works fine with your patch for example:
close all, text(0.5, 0.5, {'Hello'})
But single line texts aren't necessarily a cell array :( Plus: Even multi-line texts can still be a character array. This one should also be preserved to work (and doesn't with this first patch):
close all, text(0.5, 0.5, ['Hello'; '12345'])
Suggestion: Could you maybe change this patch, such that it starts to look for emtpy strings (using the cell array indexing, that currently throws an error) ONLY IF the whole text object is surely a cell array? In all other cases (i.e. having character arrays) there cannot be an empty line in there anyways, because array cannot have missing elements! So there is nothing to fix for those other cases with character arrays.
|