dattalab / moseq2-extract

Here it comes...
Other
3 stars 4 forks source link

Bug using --bg-roi-depth-range and --autoset-depth-range #125

Closed versey-sherry closed 3 years ago

versey-sherry commented 3 years ago

@wingillis just found that --bg-roi-depth-range should never equal to auto and the current setup may make the GUI tool unable to auto-detect parameters to generate config.yaml for ROI widget.

Since --bg-roi-depth-range is set to (650, 750) by default, and the type is expected to be (float, float), the flag should never equal a string. The current if statement shows that when autoset_depth_range is False, the depth will be auto-detected, but the auto-detection should happen when autoset_depth_range==True, which is more intuitive.

In the GUI, config.yaml is generated from default parameters from CLICK, therefore, the config.yaml needs to specify that the depth to be auto-detected.

Suggested changes:

  1. moseq2_extract/cli.py line 61 function = click.option('--autoset-depth-range', is_flag=True, -> function = click.option('--autoset-depth-range', is_flag=True, default=True
  2. When using the GUI function, the flag is set up in such a way that the depth can be automatically detected. When using the CLI, when there is a user input for --bg-roi-depth-range, the --autoset-depth-range is set to False and the user input range is used.

Updated suggested changes: Instead of click.option('--autoset-depth-range', is_flag=True, maybe it's a better idea to click.option('--manualset-depth-range', is_flag=True,, the flag will default to False to trigger the automatic depth detection for GUI. Then when users specify '--manualset-depth-range', the bg-roi-depth-range will be set to user input value, and this option should have no default value.

aymanzay commented 3 years ago

Solved in PR #126