search

microscope-cockpit / cockpit

Cockpit is a microscope graphical user interface. It is a flexible and easy to extend platform aimed at life scientists using bespoke microscopes.
https://microscope-cockpit.org
GNU General Public License v3.0
37 stars 27 forks source link

new format to specify camera transform #839

Open carandraug opened 1 year ago

carandraug commented 1 year ago

On yesterday's meeting we discussed that the format for transform on depot.conf was a bit tricky to get right because everywhere we refer to their values as bools but when reading the conf file we expect integers. We decided that we would use the same format as Python Microscope (a named tuple with bool values) and deprecate the previous. See https://github.com/python-microscope/meetings/blob/main/2023-01-10.md

carandraug commented 1 year ago

I've prepared a patch that makes it parse a string like (lr=True, ud=False, rot=True). See my branch carandraug/cockpit/839-camera-transform-new-config-format

We only discussed changing the string format to expect bools instead of ints but I thought that using a named tuple would make things clearer. Despite the use of named values it does snot support changing their order, it requires the values to be named, and is not that flexible with optional whitespace. We could add all of that but I think it will only make the regex more complicated for very little gain.

iandobbie commented 1 year ago

How about we strip white space from the string before the match. Should be easy and saves about 20 possible places that white space could go and clutter up the regexp.

I am also concerned that somewhere in the config file reading there is a tolower so it might not match to "True". I guess you have tested that.

carandraug commented 1 year ago

How about we strip white space from the string before the match. Should be easy and saves about 20 possible places that white space could go and clutter up the regexp.

That would match things (lr=T r u e, ud= Fal se, rot= F alse). We can just add a bunch of \s* on the right places if you really want to add that flexibility.

I am also concerned that somewhere in the config file reading there is a tolower so it might not match to "True". I guess you have tested that.

configparser does tolower but only on the keys so this should be fine. I've also added a bunch of test cases on the testsuite but it only tests the function, not the whole thing from config file to the parsing.

iandobbie commented 1 year ago

If we are moving to the named tuple, should that also go over the wire to microscope like the ROI does? This requires chnages to every camera module but probably worth it.