quantumjot / btrack

Bayesian multi-object tracking
https://btrack.readthedocs.io
MIT License
310 stars 50 forks source link

[BUG] Default config name is used when saving config from napari plugin #389

Open quantumjot opened 11 months ago

quantumjot commented 11 months ago

When exporting the config from the napari plugin, the default config name is used, which means it cannot be loaded until it has been renamed.

167 if config_name in self.configs and not overwrite:
    168     _msg = (
    169         f"Config '{config_name}' already exists - "
    170         "config names must be unique."
    171     )
--> 172     raise ValueError(_msg)
        _msg = "Config 'cell' already exists - config names must be unique."
    174 self.configs[config_name] = config
    176 return config_name
paddyroddy commented 11 months ago

When you click "Save configuration" it asks you to select a name. What you mean is if you select the name cell.json then you can't then load that in? Considering we have cell and particle as default ones, what exactly would you want doing? We have to have a name for those. We intentionally raise that error here: https://github.com/quantumjot/btrack/blob/2ec77286676c5fb65458f7d4fa59beb089906a2e/btrack/napari/config.py#L166-L172

quantumjot commented 11 months ago

I think it needs to be set in the actual config.json file too, rather than just the filename:

https://github.com/quantumjot/btrack/blob/2ec77286676c5fb65458f7d4fa59beb089906a2e/btrack/config.py#L21-L27

paddyroddy commented 11 months ago

Sorry, I still don't follow

quantumjot commented 11 months ago

The name attribute of the TrackerConfig needs to be updated to match the new filename, before exporting it. If it's left as cell or particle the user can't load the saved config as those already exist as default options the plugin.

paddyroddy commented 11 months ago

Okay I get you now

p-j-smith commented 11 months ago

just linking to a related issue (though not the same) https://github.com/quantumjot/btrack/issues/243

p-j-smith commented 11 months ago

it would probably be good to handle config name clashes in the plugin too - someone still might save cell.json, in which case they couldn’t load the config. Maybe a dialogue could popup asking whether the current config should be overwritten, or if the name of the loaded one should be modified. Or the name could be modified automatically

paddyroddy commented 11 months ago

Attempted but ran out of time #393