pombase / canto

The PomBase community curation tool
https://curation.pombase.org
Other
19 stars 7 forks source link

Allow transfer of interaction annotation to multiple features #2244

Closed kimrutherford closed 4 years ago

kimrutherford commented 4 years ago

See: https://github.com/pombase/canto/issues/1830#issuecomment-590839270

kimrutherford commented 4 years ago

After discussion we decided to add this transfer feature for phenotype_interaction first as it's the simple case. I hope I'm remembering that correctly.

I've started work on this (e9ca0fde6, #2245). So far it only allows transferring the annotation to genotype A of an interaction (it was the easy case). I've just added that change to the flybase-test Canto.

@vmt25 @gm119

vmt25 commented 4 years ago

phenotype_interaction first as it's the simple case. I hope I'm remembering that correctly.

Yes. This should be the easiest to implement because there are no rules on which genotypes can go with which.

Important point: Only enable changing genotype A or genotype B, never both.

vmt25 commented 4 years ago

I've started work on this (e9ca0fd, #2245). So far it only allows transferring the annotation to genotype A of an interaction (it was the easy case). I've just added that change to the flybase-test Canto.

Works great!

vmt25 commented 4 years ago

From today's VC with Kim and Gillian:

A partial implementation that allows transferring to other Interactor A genotypes while keeping the interactor B is working fine on https://curation.pombase.org/flybase-test/ (as in #2245) and would already provide some added value, so it would be good if pushed into production in Fly-Canto after a couple of changes.

Screen Shot 2020-05-05 at 15 30 48

a) change the title of the menu page to a more generic 'Transfer genetic interaction annotation'

b) show the annotation to transfer (with the title 'interaction annotation to transfer')

Screen Shot 2020-05-05 at 15 32 47

c) then list the possible interactor A genotypes where to transfer (as is, but eliminating the genotype of the original annotation)

vmt25 commented 4 years ago

(For future implementation) A more common/useful case would be to transfer an interaction annotation, where interactor B ois the the genotype to be changed.

For this, the menu/pipeline will need to select which genotype to change (interactor A or B) and, depending on the selection, list the possible genotypes (excluding the corresponding genotype on the original annotation)

jseager7 commented 4 years ago

@kimrutherford In case you don't already know, the Transfer link isn't disabled when you only have one feature in the session; you get an empty dialog box in this case:

image

kimrutherford commented 4 years ago

Thanks James. I had forgotten about that. It's on the to-do list but I had forgotten there was a to-do list. :-)

https://github.com/pombase/canto/issues/1830#issuecomment-587362776

jseager7 commented 4 years ago

@kimrutherford Not sure which issue this belongs in, but I've noticed a bug with annotation_transfer.html.

The annotation_transfer.html template uses the multi-feature-chooser directive, which has a featureType binding of string type (@), but the template code treats the binding like a variable-type (= or <), since it passes the name of a scope variable featureType as a raw string instead of as an interpolation:

      <multi-feature-chooser ng-if="!!otherFeatures" 
                             features="otherFeatures" feature-type="featureType"
                             selected-feature-ids="selectedFeatureIds">
      </multi-feature-chooser>

This fixes the value of featureType to "featureType", instead of the actual type of the feature, so the following conditional markup in multi_feature_chooser.html never shows:

  <div ng-if="featureType == 'gene'" class="clearall">
    <a ng-click="openSingleGeneAddDialog()">Add another gene</a>
  </div>

Using interpolation is all that's required to fix this:

feature-type="{{featureType}}"
kimrutherford commented 4 years ago

Using interpolation is all that's required to fix this:

Thanks James.

I made that fix then found that the "Add another gene" wasn't working. I've fixed that now but it required a slight code shuffle. I'm not sure the "Add another gene" link is that useful for Multi-organism/UniProt ID mode. We can hide it if it's not useful.

jseager7 commented 4 years ago

I'm not sure the "Add another gene" link is that useful for Multi-organism/UniProt ID mode. We can hide it if it's not useful.

It's maybe not as user-friendly as PomBase, because you can't just enter gene names, and you can't restrict the gene being added to the selected organism. The latter reason is why we replaced the shortcut in pathogen-host mode: previously, if you used the shortcut to add a gene for an organism other than the one that was selected, you didn't get any visual feedback that anything had happened. Now the Genotype Management page has a link back to the gene entry page instead.

Some of the above problems don't apply for transferring annotations, since you see all of the available features from all organisms (so you'd always see the gene that gets added). I think having the option to add a gene before transferring annotations is useful for the case where you want to copy the annotation, but forgot to add the gene in the first place.

All of this assumes the shortcut can actually work for UniProt IDs.

kimrutherford commented 4 years ago

I've had a first try at allowing annotation transfer for interactor A or B. I haven't yet implemented the suggestions in https://github.com/pombase/canto/issues/2244#issuecomment-624111797 but I've updated the main fly-canto instance with the changes so you can start testing.

What should the text be instead of "Choose interactor"? I couldn't think of a good way to describe things.

transfer-interactions-1

kimrutherford commented 4 years ago

a) change the title of the menu page to a more generic 'Transfer genetic interaction annotation' b) show the annotation to transfer (with the title 'interaction annotation to transfer')

It now looks like this: transfer-interactions-2

Is that close to what you were thinking of?

This change is in the main fly-canto and the test fly-canto.

vmt25 commented 4 years ago

Hi. The full transfer is working fine. Yes, that is what I we were thinking of, but could both a) and b) show on the previous menu instead, where the user can pick the interactor? i.e. here: image

kimrutherford commented 4 years ago

How's this?:

transfer-interactions-3

That change is in Fly-Canto now.

vmt25 commented 4 years ago

Hmmm, I don't see it... Tried Fly-Canto, both main and test

It should be the initial menu window, correct?

On the main Fly-canto I only see this on the first menu:

Screenshot 2020-07-08 at 20 41 04

And this on the second menu:

Screenshot 2020-07-08 at 20 43 10
vmt25 commented 4 years ago

How's this?:

Forgot to say that, yes, that is exactly it!

kimrutherford commented 4 years ago

Hmmm, I don't see it... Tried Fly-Canto, both main and test

Sorry, I hadn't updated fly-canto with the change. Could you try again?

vmt25 commented 4 years ago

Looks good now, Thanks!

Maybe add a header ('Interactor A' or 'Interactor B', depending on the selection) to the genotype's list on the second menu? This would provide a check for when the wrong interactor has been selected by mistake

vmt25 commented 4 years ago

Just noticed that the lists of genotypes from where to select new interactors include the initial genotype; see screenshot below. This allows for unintentional duplicated annotations - I just made one annotation but luckily noticed it.

Please notice that the list of genotype for possible interactors A includes 'FIG4[KK102943] Scer\GAL4GMR.PU', which is the interactor A on the starting annotation; the list for interactor B shows the same behavior.

Screenshot 2020-07-13 at 12 37 14
kimrutherford commented 4 years ago

Just noticed that the lists of genotypes from where to select new interactors include the initial genotype;

Hi Vitor. I think I've fixed that. Please let me know if it's still a problem.

Maybe add a header ('Interactor A' or 'Interactor B', depending on the selection) to the genotype's list on the second menu?

That makes sense. I'll do that next.

vmt25 commented 4 years ago

Just noticed that the lists of genotypes from where to select new interactors include the initial genotype;

Hi Vitor. I think I've fixed that. Please let me know if it's still a problem.

Hi Kim, At a first glance it seems to be fixed. Thanks.

kimrutherford commented 4 years ago

Maybe add a header ('Interactor A' or 'Interactor B', depending on the selection) to the genotype's list on the second menu?

I've added that now. It looks like this: interaction-header-1

Let me know if you'd like it worded differently. I don't think it's very clear at the moment.

vmt25 commented 4 years ago

Looks good! (Will check on my next curation opportunity)

Would it be possible to have the 'Interactor A'/'Interactor B' in bold and start with a capital 'I'?

Thanks

kimrutherford commented 4 years ago

Would it be possible to have the 'Interactor A'/'Interactor B' in bold and start with a capital 'I'?

Like this?:

interaction-header-2

vmt25 commented 4 years ago

Yes!!

kimrutherford commented 4 years ago

Great. That change is available in fly-canto now.

vmt25 commented 4 years ago

I think this is done - closing