legumeinfo / jira-issues

placeholder repo for issues migrating from JIRA system, to be moved to their appropriate places later
0 stars 0 forks source link

Phylogram view: AHRD descriptors in guide text at top #388

Closed adf-ncgr closed 7 years ago

adf-ncgr commented 9 years ago

Phylotree module, phylogram view: the guide text at top has AHRD descriptor (the canonical gene model and the quality score). This sort of appears to be abrupt (my impression only) marks to the smooth flowing guide text. Perhaps needs smoothing the text to make a better flow of reading.

More details from Andrew:
Those gene model names and *- is part of the AHRD output; the gene descriptors have them
too (the ones displayed here are the descriptors associated with the gene family consensus sequences).
The listed gene model is the gene model from which the functional description was taken, and the *- string represents a code regarding quality (explained more here https://github.com/groupschoof/AHRD - section 2.4.1). It is likely to be confusing to people without further explanation (or perhaps even with).

[LEGUME-420] created by sdash-legume

adf-ncgr commented 9 years ago

How about a minor modification something similar to below.

From:
"..... GO:0005524 (ATP binding), GO:0006468 (protein phosphorylation) -* gi|351721933|ref|NP_001237994.1|"

To:
"..... GO:0005524 (ATP binding), GO:0006468 (protein phosphorylation), AHRD score for consensus seq (-*), description from (gi|351721933|ref|NP_001237994.1|)"

And let the message evolve while keeping it succint.

by sdash-legume

adf-ncgr commented 7 years ago

Andrew Farmer are you good with the modification proposed by Sudhansu Dash or do you want to give me some additional instruction? Thanks

by agr

adf-ncgr commented 7 years ago

Alex & Andrew:
Also a note somewhere suitable on what does the *s and -s mean from AHRD. Andrew had promised to provide that info to you.( If he is busy, I can dig it out and Andrew can later modify it. Let me know.)

by sdash-legume

adf-ncgr commented 7 years ago

here's the info on AHRD quality (looks like in newer versions of AHRD they have dropped the fourth component of quality, but we're still using this one):

AHRD's quality-code consists of a four character string, where each character
is either '' if the respective criteria is met or *'-'
otherwise. Their meaning is explained in the following table:

Position Criteria
1 Bit score of the blast result is >50 and e-value is
2 Overlap of the blast result is >60%
3 Top token score of assigned HRD is >0.5
4 Gene ontology terms found in description line

I'm not entirely clear on how you're intending to approach this; we might want to have a conversation before you begin any sort of implementation to make sure we're all on a similar page about this.

by adf_ncgr

adf-ncgr commented 7 years ago

Really this seems like an issue of the gene family pipeline/loading itself. Because all the phylotree module does, is print the phylotree.comment field into the tripal template.

I can try to parse the comment to change the display, however it seems like more of a db loading issue. Let me know what you all think.

However, I can work un adding a popup dialog or some kind of help to describe the AHRD codes (like the table shown by andrew)

by agr

adf-ncgr commented 7 years ago

A simple 'tooltip' of Andrew's table on hover the symbols

(**-*-)

should do a good job helping the user what it means.

by sdash-legume

adf-ncgr commented 7 years ago

OK I can certainly do that in javascript. Then no database changes are needed.

by agr

adf-ncgr commented 7 years ago

I agree with Alex Rice concerning this probably being more a db-loading level thing. And if we want to maintain the idea that the module might be used by someone else, just printing the comment as stated seems appropriate. In some sense, it seems like we might just put some of this
info in the "analysis" record that describes the construction of the gene families and trees (this would just extend the description to the annotation of the gene family consensus). A bit hidden there, perhaps.

note that we basically have the same problems in the gene descriptors. so a solution that
worked reasonably well in both contexts would be ideal. not sure how much effort it merits, though. If you have a simple approach that we can revisit in future if we feel the urge, just go ahead.

by adf_ncgr

adf-ncgr commented 7 years ago

I made a javascript-only solution that is on lis-dev right now if you want to give it a try. It matches on

/[\*\-]

{3,4}

/

So I think it should work for either the older or newer format of the code.

by agr

adf-ncgr commented 7 years ago

actually just updated to use Sudhansu's descriptive text

by agr

adf-ncgr commented 7 years ago

thanks, looks pretty good. I might make a few tweaks at some point, e.g. it's never explained what "AHRD" means; maybe just hyperlinking to https://github.com/groupschoof/AHRD would be sufficient.
Also, we should be consistent what we call the thing being symbolized in the various bits of the UI-
I like "AHRD code" for its compactness- "AHRD score" is OK too, but they refer to it as "quality-code" so following their terminology a bit more closely might be slightly preferable. If you feel like doing the tweaking I won't mind.

Now that we've started down the slippery slope, I seem to recall there may be another JIRA issue requesting hyperlinking of the IPR accessions in these descriptors, maybe the GO accessions too. It may have been in the context of the gene descriptors, though. not a burning issue, but if you have time to look at it and it isn't adding too much additional javaspaghetti, it might be worth a little additional time.

thanks!

by adf_ncgr

adf-ncgr commented 7 years ago

OK I made those changes on lis-dev, please check it out.

re: linking out to IPR accessions and GO accessions, I am not sure what that would involve or where the links would go to. Create another Jira issue? I cannot find an existing one that seems to be about this.

by agr

adf-ncgr commented 7 years ago

Just checked and the explanation along with the table seems sufficient to give an idea to the user what this cryptic symbol means in this context.
A very minor issue:
The AHRD link serves a very good purpose too, however, it hijacks the LIS site to theirs as the link opens in the same window. The external site should open in another tab/window (target=_blank) and we have almost consistently followed (or agreed to follow) this policy for LIS/PB except in very old links and cases of oversight.

by sdash-legume

adf-ncgr commented 7 years ago

Hmm "hijacks" is a strong word in this context. Sorry, I was not aware of this policy, or I simply forgot.

Currently all of the feature linkouts in the phylotree do not use target=_blank. The tripal_linkout module should return a boolean property saying whether it is an external linkout or not, to save the client script from having to detect the hostname to try to figure out whether to add the target attribute or not. Someone want to create a new JIRA and assign to me?

by agr

adf-ncgr commented 7 years ago

Also, I disagree with the policy. Users of any browser can right-click on a link, or Cmd-click on a link if they want to pollute their environment with lots of new tabs or windows. Dont make the decision for the user.

by agr

adf-ncgr commented 7 years ago

GH-533 Done is what I was thinking of. It's formulated with respect to gene descriptors, but relevant here as well.The protein domain search /search/protein_domains
shows linking based on IPR accession (does not have to deal with parsing the descriptors in that context)

regarding the linking to external site, I tend to agree with Alex, that opening a new window should not
be imposed on the user; in the drupal site, you get a little symbolic warning that you'll be going outside
the site with the little icon to the right of the link text. So, assuming our users know how to use the browser
functionality to make the choice of new window or not, I think this is fine.

by adf_ncgr

adf-ncgr commented 7 years ago

that is to say, I wasn't aware there was a policy about this either (or forgot!)

by adf_ncgr

adf-ncgr commented 7 years ago

I guess #420 is done, unless someone wants to make an executive override about the link out attributes.

Hmm I read through #533 but having trouble making the conceptual leap from the current issue. We should talk about this in person.

by agr

adf-ncgr commented 7 years ago

This is done and updated onto lis-stage.

by agr