ome / omero-matlab

Toolbox for accessing OMERO from MATLAB
GNU General Public License v2.0
0 stars 6 forks source link

editChannelName #6

Closed joshmoore closed 5 years ago

joshmoore commented 5 years ago

Copy of https://github.com/openmicroscopy/openmicroscopy/pull/5808


What this PR does

This PR adds editChannelNames.m MATLAB function and it allows MATLAB users to retrieve or edit channel names easily.

Testing this PR

  1. required setup

You need at least one image, ideally with multiple channels, in OMERO server.

  1. actions to perform

imageID = 25; % assuming it has three channels

T = editChannelNames(session,imageID) % to retrieve the current channel names

T = editChannelNames(session,imageID,{'AMCA','Alexa488','Cy3'}) % to edit the channel names

T = editChannelNames(session,imageID,{'Alexa405','FITC'},'ChannelIDs',[21,22]) % to edit the specific subset of channel names
  1. expected observations

The first syntax will retrieve channel IDs and names as table format. The second and third syntaxes should change the channel names in OMERO server.

jburel commented 5 years ago

editChannelNames(session,imageID): output:

Id         Name   
    _____    __________

    15941    'CFP-JP4' 
    15942    'RD-TR-PE'
jburel commented 5 years ago

I select an image with 2 channels: Running editChannelNames(session,imageID,{'AMCA','Alexa488','Cy3'}) will correctly update the channels. Running editChannelNames(session,imageID,{'AMCA'}) will fail. I was expecting it to update the first channel only cc @kouichi-c-nakamura

jburel commented 5 years ago

Running editChannelNames(session,imageID,{'Alexa405','FITC'},'ChannelIDs',[21,22]) with non valid will not change anything: expected Running editChannelNames(session,imageID,{'Alexa405','FITC'},'ChannelIDs',[21,22]) with valid will change the name: expected

jburel commented 5 years ago

To test last commit select an image with more than one channel Run editChannelNames(session,imageID,{'AMCA'})

The other cases have already been tested

dominikl commented 5 years ago

Everything works like descriped, apart from:

>> T = editChannelNames(session,imageID,{'Alexa405','FITC'},'ChannelIDs',[1,2]) % to edit the specific subset of channel names

T =

  3×3 table

        Id           Name        NewName  
    __________    __________    __________

    1.4906e+05    'blup'        'blup'    
    1.4906e+05    'Alexa488'    'Alexa488'
             0    []            []    

Doesn't change the channel names. I'm a bit confused why the example in the PR description says [21,22] instead of [1,2] when stating assuming it has three channels (the image), but doesn't work with [21,22] either.

jburel commented 5 years ago

[1, 2] are the ID of the channels

dominikl commented 5 years ago

Indeed, it works with the ID 👍
RFE: Would be easier to use the channel index.

kouichi-c-nakamura commented 5 years ago

I can think of supporting another option ChannelIndex. But I'm not sure how to convert channel indices to channel IDs.

T = editChannelNames(session,imageID,{'Alexa405','FITC'},'ChannelInedx',[1,2]) % to edit the specific subset of channel names
jburel commented 5 years ago

@kouichi-c-nakamura I don't think we need both, it is certainly more "natural" to pass the channel's index than the channel's id/ From the list of channels linked to an image you can retrieve its id then follow the current implementation

kouichi-c-nakamura commented 5 years ago

OK, I'll think about it next time I use this.

jburel commented 5 years ago

@kouichi-c-nakamura Nothing has changed on the code front. Will you be looking at it?

kouichi-c-nakamura commented 5 years ago

Sorry, I'm currently working on something totally unrelated. Should this be done sooner?

dominikl commented 5 years ago

I think the PR is good to merge. Using channel index instead of id could be potential follow-up PR.

jburel commented 5 years ago

@dominikl the problem is that if we switch to using channels index instead of ids then it will be a breaking change

kouichi-c-nakamura commented 5 years ago

OK, I'll leave it to you guys. If it's better to be done sooner, I can work on it.

jburel commented 5 years ago

cc @dominikl The last commit allows to specify the channel index instead of channel ID (start at 1) Select an image with at least 2 channels to test

T = editChannelNames(session,imageID,{'AMCA','Alexa488','Cy3'}) % to edit the channel names

T = editChannelNames(session,imageID,{'Alexa405','FITC'},'channelIndexes',[1,2]) % to edit the first 2 channels

T = editChannelNames(session,imageID,{'Alexa405',},'channelIndexes',[2]) % to edit the second channel
dominikl commented 5 years ago

Looks good. Works as expected 👍 Although in the example above it should now be ChannelIndex (instead of ChannelIDs) too.

jburel commented 5 years ago

@dominikl I have updated the doc if you want to have a look

dominikl commented 5 years ago

Thanks. Looks good to merge imo 👍