Skip to content

Fix reactivate channels#40

Merged
joshmoore merged 3 commits intoome:masterfrom
will-moore:fix_reactivate_channels
Jul 9, 2020
Merged

Fix reactivate channels#40
joshmoore merged 3 commits intoome:masterfrom
will-moore:fix_reactivate_channels

Conversation

@will-moore
Copy link
Member

Previously, img.set_active_channels(reactivatechannels) was turning OFF the channels
that were active and edited in the previous set_active_channels() call.

To test, find an Image with 3 Channels, all Active.
Run:

$ omero render set Image:1 settings.yml

with less than 3 Channels specified, e.g.

settings.yml

---
channels:
    1:
        active: true
        start: 0
        end: 255
greyscale: false
version: 2

This should only edit the first Channel. All 3 channels should stay Active.

Previously, img.set_active_channels(reactivatechannels) was turning OFF the channels
that were active and edited in the previous set_active_channels() call
@will-moore will-moore changed the title Don't turn OFF channels in cindices Fix reactivate channels Jun 19, 2020
@joshmoore
Copy link
Member

joshmoore commented Jun 22, 2020

Also note the now merged update to the readme:

https://github.com/ome/omero-cli-render/pull/41/files#diff-7c3731557868b1b97ef87421dd10cf92R119

# Omitted channels will not be disabled unless --disable is used.

@sbesson
Copy link
Member

sbesson commented Jun 22, 2020

Tested using a fake image with four channels test&sizeC=4.fake as the input file with original.yml containing the original rendering settings:

---
channels:
    1:
        active: true
        color: FF0000
        end: 255.0
        label: '0'
        max: 255.0
        min: 0.0
        start: 0.0
    2:
        active: true
        color: 00FF00
        end: 255.0
        label: '1'
        max: 255.0
        min: 0.0
        start: 0.0
    3:
        active: true
        color: 0000FF
        end: 255.0
        label: '2'
        max: 255.0
        min: 0.0
        start: 0.0
    4:
        active: false
        color: FF0000
        end: 255.0
        label: '3'
        max: 255.0
        min: 0.0
        start: 0.0
greyscale: false
t: 1
version: 2
z: 1

and test.yml as suggested from the PR description

---
channels:
    1:
        active: true
        start: 0
        end: 100
greyscale: false
version: 2

Running bin/omero render set Image:123223 test.yaml && bin/omero render info Image:123223 updates the rendering window of channel 1 as expected

rdefv2: model=color, z=0, t=0
tiles: False
ch0: active=True,color=FF0000,label=0,min=0.0,start=0.0,end=100.0,max=255.0
ch1: active=True,color=00FF00,label=1,min=0.0,start=0.0,end=255.0,max=255.0
ch2: active=True,color=0000FF,label=2,min=0.0,start=0.0,end=255.0,max=255.0
ch3: active=False,color=FF0000,label=3,min=0.0,start=0.0,end=255.0,max=255.0

Running bin/omero render set Image:123223 test.yaml --disable && bin/omero render info Image:123223 disables channels 2 and 3 as expected

rdefv2: model=color, z=0, t=0
tiles: False
ch0: active=True,color=FF0000,label=0,min=0.0,start=0.0,end=100.0,max=255.0
ch1: active=False,color=00FF00,label=1,min=0.0,start=0.0,end=255.0,max=255.0
ch2: active=False,color=0000FF,label=2,min=0.0,start=0.0,end=255.0,max=255.0
ch3: active=False,color=FF0000,label=3,min=0.0,start=0.0,end=255.0,max=255.0

However, trying to restore the original YAML file with bin/omero render set Image:123223 original.yml && bin/omero render info Image:123223 still gives channels 2 and 3 as disabled

rdefv2: model=color, z=0, t=0
tiles: False
ch0: active=True,color=FF0000,label=0,min=0.0,start=0.0,end=255.0,max=255.0
ch1: active=False,color=00FF00,label=1,min=0.0,start=0.0,end=255.0,max=255.0
ch2: active=False,color=0000FF,label=2,min=0.0,start=0.0,end=255.0,max=255.0
ch3: active=False,color=FF0000,label=3,min=0.0,start=0.0,end=255.0,max=255.0

Passing bin/omero render set Image:123223 original.yml --disable && bin/omero render info Image:123223 gives the expected outcome

rdefv2: model=color, z=0, t=0
tiles: False
ch0: active=True,color=FF0000,label=0,min=0.0,start=0.0,end=255.0,max=255.0
ch1: active=True,color=00FF00,label=1,min=0.0,start=0.0,end=255.0,max=255.0
ch2: active=True,color=0000FF,label=2,min=0.0,start=0.0,end=255.0,max=255.0
ch3: active=False,color=FF0000,label=3,min=0.0,start=0.0,end=255.0,max=255.0

So it looks like there is still an issue when switching the channel state from disactive to active and --disable is not passed.

@will-moore
Copy link
Member Author

@sbesson Thanks for catching that. Should be fixed now

@sbesson
Copy link
Member

sbesson commented Jun 23, 2020

Thanks @will-moore. Quickly looking at the code, the latest logic makes sense to me i.e. activate a channel:

  • either if active: true is passed
  • or if the channel is already active and active: false is not set

Note this particular functionality has already been the object of previous work in #15. In addition to manual testing, could we look into adding an integration test covering the expected channel activation behavior with/without --disable for the scenario described in this PR?

@dominikl
Copy link
Member

I think the whole issue is actually one level higher, that one has to call set_active_channels in order change the rendering settings. De/Activating channels and modifying the various channel settings should be two separate things.

@will-moore
Copy link
Member Author

I guess that set_active_channels() behaves as if we're using a --disable flag, and you'd like an option to "ignore unlisted channels"?
I guess that could be a useful update to the BlitzGateway.
Anyway, I'll look at adding more integration tests...

@dominikl
Copy link
Member

dominikl commented Jul 9, 2020

Looks good to me 👍

@will-moore
Copy link
Member Author

"Good to merge"
Merge needed to avoid conflict with #42 - thanks

@joshmoore joshmoore merged commit fc6e4e4 into ome:master Jul 9, 2020
@sbesson
Copy link
Member

sbesson commented Jul 9, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants