SainsburyWellcomeCentre / masiv

MATLAB Streaming Image Viewer - a viewing and analysis platform for ultra-large 3D imaging data
MIT License
12 stars 5 forks source link

Regression: numeric settings are now read as character arrays #15

Open raacampbell opened 7 years ago

raacampbell commented 7 years ago

It looks like the changes for reading sample IDs starting with a number or a zero have screwed up the reading of the settings file. So it now reads numeric settings as strings which made me think I had a bug here: 83b1568f5ebe7c7dce6db979513f2fd05f6feea8 That fixed it by accident but now we can't pan because the pause setting for the keyboard pan is broken.

raacampbell commented 7 years ago

Hmmm... So it's just crap that starts with a zero.

>> m=masiv.yaml.readSimpleYAML('masivPrefs.yml');m.navigation

ans = 

  struct with fields:

            panIncrement: [10 120]
         scrollIncrement: [10 1]
                zoomRate: 1.5000
           panModeInvert: '0'
        scrollZoomInvert: 1
       scrollLayerInvert: 1
    keyboardUpdatePeriod: '0.005'
raacampbell commented 7 years ago

I'll do this. I'll scale the heights of neat software development by doing it with a unit test.

alexanderbrown commented 7 years ago

Argh I knew what I was doing was hacky. On Wed, 11 Jan 2017 at 09:38, Rob Campbell notifications@github.com wrote:

I'll do this. I'll scale the heights of neat software development by doing it with a unit test.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/alexanderbrown/masiv/issues/15#issuecomment-271822114, or mute the thread https://github.com/notifications/unsubscribe-auth/AFH45e4Utgt0wBef3w0Vt4iR3bD540PHks5rRKMJgaJpZM4LgWL1 .

raacampbell commented 7 years ago

I'm on it right now.

alexanderbrown commented 7 years ago

Right. This will of course have been caused by the fix for the sample ID. Anything numeric with a leading zero is kept as a string by readSimpleYAML. Perhaps a fix is to change this to anything numeric, which is not in fact equal to zero? On Wed, 11 Jan 2017 at 10:03, Rob Campbell notifications@github.com wrote:

I'm on it right now.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/alexanderbrown/masiv/issues/15#issuecomment-271827647, or mute the thread https://github.com/notifications/unsubscribe-auth/AFH45fGacqCL-y80hDfDjvNezn2bVdPgks5rRKjjgaJpZM4LgWL1 .

raacampbell commented 7 years ago

Yes, that seems reasonable.

raacampbell commented 7 years ago

Except that this won't catch the problematic YAML that we were trying to fix, since there stackName was equal to 01 with no characters. I think the only way to fix that would be to force it to read certain settings as a settings (or certain files) a string.

alexanderbrown commented 7 years ago

Or to force the user to escape the stackname e.g. '01' in the file, and detect this On Wed, 11 Jan 2017 at 10:11, Rob Campbell notifications@github.com wrote:

Except that this won't catch the problematic YAML that we were trying to fix, since there stackName was equal to 01 with no characters. I think the only way to fix that would be to force it to read certain settings as a settings (or certain files) a string.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/alexanderbrown/masiv/issues/15#issuecomment-271829580, or mute the thread https://github.com/notifications/unsubscribe-auth/AFH45c0EH63Iom1VxI3f1CnkTHnNXyYaks5rRKrtgaJpZM4LgWL1 .

raacampbell commented 7 years ago

The saga continues: fd8fb9e. I think we're now good. It passes the tests and MaSIV works again.

alexanderbrown commented 7 years ago

Looks good, makes sense.

Logically, there should be no number, saved by matlab, that is a whole number, with leading zeros.

He says, fingers crossed.