Param ui #119

k1o0 · 2019-03-11T14:34:05Z

Added new integrated parameter editor
Added test class for editor
Added test class for Parameters
Added performance test
Comment the ParamEditor, FieldPanel and ConditionTable classes
Deleted ParamEditor_old
Restored param description and unit functionality
Restored label formatting

todo · 2019-03-11T14:34:20Z

Add sort by column

Lines 5 to 10 in 9babd51

    
           % TODO Add sort by column 
        
           % TODO Add set condition idx 
        
           % TODO Use tags for menu items 
        
           properties 
        
             ConditionTable

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

todo · 2019-03-11T14:34:23Z

Add set condition idx

Rigbox/+eui/ConditionPanel.m

Lines 6 to 11 in 9babd51

    
           % TODO Add set condition idx 
        
           % TODO Use tags for menu items 
        
           properties 
        
             ConditionTable 
        
             MinWidth = 80

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

todo · 2019-03-11T14:34:32Z

. See also EXP.PARAMETERS.

Rigbox/+eui/ParamEditor_old.m

Lines 3 to 8 in 9babd51

    
           %   TODO. See also EXP.PARAMETERS. 
        
           % 
        
           % Part of Rigbox 
        
           % 2012-11 CB created   
        
           % 2017-03 MW/NS Made global panel scrollable & improved performance of

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

todo · 2019-03-12T12:57:36Z

Maybe use exp.Parameters/ui

Rigbox/+eui/FieldPanel.m

Lines 38 to 43 in 29266f0

    
           % TODO Maybe use exp.Parameters/ui 
        
           if isempty(obj.ContextMenu) 
        
             obj.ContextMenu = uicontextmenu; 
        
             uimenu(obj.ContextMenu, 'Label', 'Make Conditional', ... 
        
               'MenuSelectedFcn', @(~,~)obj.makeConditional); 
        
           end

This comment was generated by todo based on a `TODO` comment in `29266f0` in #119. cc @cortex-lab.

… test for repelems

k1o0 · 2019-03-13T20:25:32Z

Building new ParamEditor window with parameters loaded takes < 0.4s and to load in a new set of parameters < 0.3s
There are numerous bug fixes uncovered by the test class
numRepeats can now be a global parameter. This changes the way the trial conditions are repeated giving the user more flexibility
Trial conditions can now be fixed, i.e. the user can choose not to randomize the trials within the GUI
There are now tests for the ParamEditor class and Parameters class
ParamEditor and Parameters classes are more thoroughly documented

Still left to do for another day:

Properly integrate string params: currently they're converted to char when made conditional. This isn't necessary.
Add test for set values method
Add method for sorting trial conditions by a column
Add method for setting specific trial condition order (i.e. adding conditionIdx param functionality into SignalsExp)

jkbhagatio · 2019-03-14T15:55:53Z

+eui/ConditionPanel.m

+      % See also EUI.PARAMEDITOR, EUI.FIELDPANEL
+      obj.ParamEditor = ParamEditor;
+      obj.UIPanel = uix.VBox('Parent', f);
+%       obj.UIPanel.BackgroundColor = 'white';


can this be removed?

You may recall that the editor was originally white, which I think looks nicer. MC is still all grey by default. I can't be arsed to change everything to white so the short answer is yes, let's remove all of these BackgroundColor lines.

ha hmm well you're right the white background does look nicer...

jkbhagatio · 2019-03-14T16:05:03Z

+eui/FieldPanel.m

@@ -0,0 +1,229 @@
+classdef FieldPanel < handle


why call this 'FieldPanel' instead of something like 'GlobalPanel'?

This is what Chris originally called it. It's a panel that generates UI fields. I suppose it's more functionally clear than 'global' which is a less concrete name

jkbhagatio · 2019-03-14T16:06:01Z

+eui/MControl.m

@@ -247,10 +247,11 @@ function saveParamProfile(obj) % Called by 'Save...' button press, save a new pa
    end

    function loadParamProfile(obj, profile)
+      tic


can the 'tic' and subsequent 'toc' be removed?

Shit, yes. I think there's a toc at the bottom of the script. This was done before I wrote the performance test class

jkbhagatio · 2019-03-14T16:19:39Z

tests/ParamEditorPerfTest.m

+      % Loads validation data
+      %  Graph data is a cell array where each element is the graph number
+      %  (1:3) and within each element is a cell of X- and Y- axis values
+      %  respecively


jkbhagatio

ok, yeah just really minor fixes for you as commented above. Also, did you not want to include tests specifically for 'ConditionPanel' and 'FieldPanel' ? and can we keep the '_test' convention even for class tests?

also, 'ParamEditorTest' and its perf test failed. I put them the test results variable in your 'zserver' 'share' folder. The first 15 elements of the variable are the ones that failed.

k1o0 · 2019-03-14T17:26:54Z

The ConditionPanel and FieldPanel classes aren't really standalone so standalone tests aren't useful. All the functionality of these two classes is tested by the ParamEditor test.
Okay
These errors aren't really informative, could we run then test together tomorrow?

k1o0 · 2019-03-14T19:11:09Z

Before merging, let's make sure the tests run on a real rig, double-check everything works properly within mc and also delete the +eui/ParamEditor_old.m file.

k1o0 · 2019-03-14T19:13:45Z

Oh and the .github/config.yml didn't seem to do anything. I suppose we just delete it, unless I made a typo somewhere?

jkbhagatio · 2019-03-14T19:41:30Z

Ok, I may have time to test it on the behaviour rigs after upgrading the hardware, while I'm installing the new MATLABs. Would late afternoon work for you? Yeah the .yml file looks right... I'll also look into it

jkbhagatio · 2019-03-15T23:00:53Z

+eui/ConditionPanel.m

+  end
+
+  methods
+    function obj = ConditionPanel(f, ParamEditor, varargin)


why is there a varargin arg here?

jkbhagatio · 2019-03-15T23:14:11Z

I've reorganized the test folder structure and think I found a way to safely test with "dat.paths" (and any other folder/file we may need to shadow"). Let me know what you think

k1o0 · 2019-03-18T12:47:24Z

Example of shared fixtures. All it really affords you is fewer lines of code as you don't have to manually add the taredown routine.

… test classes

jkbhagatio · 2019-03-20T10:45:25Z

ok, i will merge this now, but keep parameditor_old around, and keep this branch open

Param ui into dev

k1o0 added 5 commits January 31, 2019 00:38

Attempt to integrate new Parameter Editor into mc

b36cadb

Updates for GUILT compatibility

04a8a3d

Merge branch 'dev' of https://github.com/cortex-lab/Rigbox into paramUI

67e3de3

Updates to signals

f93f73b

Added config file for todo bot

9babd51

k1o0 added the enhancement label Mar 11, 2019

k1o0 requested a review from jkbhagatio March 11, 2019 14:34

Hack for resize issue and fix for makeConditional()

09deaf1

Added some tests and minor modifications

29266f0

k1o0 added 4 commits March 12, 2019 21:37

Complete Parameters test; parameters deals with strings better; added…

ad16e6f

… test for repelems

Bug fixes for numRepeats behaviour, more tests

57ba45c

Better coverage of Changed event

335642a

Added full documentation

d9fc48d

cortex-lab deleted a comment from todo bot Mar 13, 2019

Added performance test and tooltip strings

8272c01

k1o0 marked this pull request as ready for review March 13, 2019 20:14

This was referenced Mar 13, 2019

remove unnecessary hbox #16

Closed

mc takes forever to open #10

Closed

jkbhagatio reviewed Mar 14, 2019

View reviewed changes

jkbhagatio requested changes Mar 14, 2019

View reviewed changes

Removed some comments and command promt output, renamed tests

53e23fe

k1o0 requested a review from jkbhagatio March 14, 2019 19:09

Changed RearrangeableColumns from true to 'on'

c8f7eb6

jkbhagatio reviewed Mar 15, 2019

View reviewed changes

+eui/ConditionPanel.m

end

methods

function obj = ConditionPanel(f, ParamEditor, varargin)

Copy link

Member

jkbhagatio Mar 15, 2019

Choose a reason for hiding this comment

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

why is there a varargin arg here?

reorganized test folder structure, plus added some comments

d1ded06

jkbhagatio force-pushed the paramUI branch from 24b466d to d1ded06 Compare March 15, 2019 23:07

renamed folder 'helpers' to 'fixtures'; added 'SharedTestFixtures' to…

4c9b67f

… test classes

jkbhagatio approved these changes Mar 20, 2019

View reviewed changes

jkbhagatio merged commit b85f8f8 into dev Mar 20, 2019

This was referenced Mar 20, 2019

[BUG] Missed todo creation when merging PR JasonEtco/todo#186

Closed

Suggestion: increase the diff limit a bit JasonEtco/todo#198

Closed

k1o0 pushed a commit that referenced this pull request Jan 3, 2021

Merge pull request #119 from cortex-lab/paramUI

6fdeb75

Param ui into dev

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Param ui #119

Param ui #119

k1o0 commented Mar 11, 2019 •

edited

Loading

todo bot commented Mar 11, 2019

todo bot commented Mar 11, 2019

todo bot commented Mar 11, 2019

todo bot commented Mar 12, 2019

k1o0 commented Mar 13, 2019

jkbhagatio Mar 14, 2019

k1o0 Mar 14, 2019

jkbhagatio Mar 14, 2019

jkbhagatio Mar 14, 2019

k1o0 Mar 14, 2019

jkbhagatio Mar 14, 2019

k1o0 Mar 14, 2019

jkbhagatio Mar 14, 2019

jkbhagatio left a comment

k1o0 commented Mar 14, 2019

k1o0 commented Mar 14, 2019

k1o0 commented Mar 14, 2019

jkbhagatio commented Mar 14, 2019

jkbhagatio Mar 15, 2019

jkbhagatio commented Mar 15, 2019

k1o0 commented Mar 18, 2019

jkbhagatio commented Mar 20, 2019

Param ui #119

Param ui #119

Conversation

k1o0 commented Mar 11, 2019 • edited Loading

todo bot commented Mar 11, 2019

Add sort by column

This comment was generated by todo based on a TODO comment in 9babd51 in #119. cc @cortex-lab.

todo bot commented Mar 11, 2019

Add set condition idx

This comment was generated by todo based on a TODO comment in 9babd51 in #119. cc @cortex-lab.

todo bot commented Mar 11, 2019

. See also EXP.PARAMETERS.

This comment was generated by todo based on a TODO comment in 9babd51 in #119. cc @cortex-lab.

todo bot commented Mar 12, 2019

Maybe use exp.Parameters/ui

This comment was generated by todo based on a TODO comment in 29266f0 in #119. cc @cortex-lab.

k1o0 commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jkbhagatio left a comment

Choose a reason for hiding this comment

k1o0 commented Mar 14, 2019

k1o0 commented Mar 14, 2019

k1o0 commented Mar 14, 2019

jkbhagatio commented Mar 14, 2019

Choose a reason for hiding this comment

jkbhagatio commented Mar 15, 2019

k1o0 commented Mar 18, 2019

jkbhagatio commented Mar 20, 2019

k1o0 commented Mar 11, 2019 •

edited

Loading

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

This comment was generated by todo based on a `TODO` comment in `9babd51` in #119. cc @cortex-lab.

This comment was generated by todo based on a `TODO` comment in `29266f0` in #119. cc @cortex-lab.