Back to the main page.

Bug 3146 - translate vlim and hlim in multiplot to ylim and xlim

Status NEW
Reported 2016-06-14 17:23:00 +0200
Modified 2016-09-13 14:10:28 +0200
Product: FieldTrip
Component: plotting
Version: unspecified
Hardware: PC
Operating System: Mac OS
Importance: P5 normal
Assigned to:
URL:
Tags:
Depends on:
Blocks:
See also:

Robert Oostenveld - 2016-06-14 17:23:53 +0200

Seyed reported on https://github.com/fieldtrip/fieldtrip/pull/174 that vlim and hlim in multiplot are translated to ylim and xlim in singleplot when the interactive option is used. This could be advantageous when the multiplotER is used interactively. In the previous version even if zlim, vlim, and hlim are set in the call of multiplotER, they are not forwarded correctly to the interactively called singleplotER and the subsequent topoplotER. For example, this is problematic especially when you want to have a 'maxabs' limited topoplot. Even if the vlim and zlim are set to 'maxabs' before calling multiplot, in the subsequent topoplot which is interactively plotted, the zlim is still 'minmax'. The simple change proposed by this pull request solves the problem.


Robert Oostenveld - 2016-06-14 17:25:31 +0200

I responded hmm, although I understand why you want this change, I have the feeling that it would move us in the wrong direction. The reason for moving away from xlim/ylim/zlim is that x/y/z are too confusing to label 3D data, and especially when the code needs to support 2D and 3D. The multiplot and singleplot figures technically have a horizontal and vertical axis. And we use color (in case of 3D). I remember that we tried at a certain point to move away from x/y/z, but the code is not consistent in this respect.


Robert Oostenveld - 2016-06-14 17:27:39 +0200

should we not replace xlim/ylim by hlim/vlim instead, i.e. opposite to the suggested change? Why are we using zlim in some of the code instead of clim?


Seyed Mostafa Kia - 2016-06-14 17:59:33 +0200

As mentioned by Robert, there is a higher level problem in the code (above the specific case mentioned in my pull-request), where there is an inconsistency in naming different dimensions of the data. While in some functions the x/y/z convention is employed, others stick to h/v/c. This could be problematic when these two different conventions have conflict or interaction (for example when a sequence of interactive plots are drawn). I can understand the challenge of resolving this inconsistency and keeping the other components intact at the same time.


Robert Oostenveld - 2016-09-13 14:01:43 +0200

(In reply to Seyed Mostafa Kia from comment #3) I came across https://github.com/fieldtrip/fieldtrip/pull/174 which triggered me to review parts of the code. At this moment fieldtrip/plotting is exclusively using him/vlim/clim (horizontal, vertical, color). In the high-level plotting functions (in fieldtrip main directory) I primarily see xlim/ylim/zlim. The documentation (and hence possibly the code) of the following high-level functions deviates from this mac011> grep '%.*hlim' *.m ft_databrowser.m:range = range * (opt.hlim(2) - opt.hlim(1)) + opt.hlim(1); % 0 becomes hlim(1), 1 becomes hlim(2) ft_databrowser.m:% the hlim will be in seconds, the vlim will be in Tesla or Volt ft_multiplotER.m:% cfg.hlim = 'maxmin' or [xmin xmax] (default = 'maxmin') mac011> grep '%.*vlim' *.m ft_databrowser.m: % this is a lot easier than the reverse, determining the y value of each time course scaled by the layout and vlim ft_databrowser.m:% the hlim will be in seconds, the vlim will be in Tesla or Volt ft_multiplotER.m:% cfg.vlim = 'maxmin', 'maxabs', 'zeromax', 'minzero', or [ymin ymax] (default = 'maxmin') ft_multiplotER.m:% Get physical y-axis range (vlim / parameter): For clim/zlim it is more difficult, so I'll skip that for now.


Robert Oostenveld - 2016-09-13 14:10:28 +0200

ft_databrowser is not a problem, i.e. there is no end-user reference to hlim/vlim ft_multiplotER is a problem, as it is inconsistent with ft_singleplotER. I changed that function to make it consistent. mac011> git commit -a [bug3146-xlimhlim bef460e] ENH - make xlim/ylim consistent throughout the high-level code, do not use hlim/vlim. See http://bugzilla.fieldtriptoolbox.org/show_bug.cgi?id=3146 1 file changed, 19 insertions(+), 19 deletions(-)