Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trim tooltip #20

Open
wants to merge 16 commits into
base: staging-wp5
Choose a base branch
from
Open

Trim tooltip #20

wants to merge 16 commits into from

Conversation

russHyde
Copy link
Collaborator

@russHyde russHyde commented Dec 7, 2022

The text that is displayed when the user mouse-overs a node on the treeview contained lots of information.
Here, we trim down the text to just display the cluster ID for the hovered node.

The original code (in append_interactivity_data) made a lot of changes to the "$data" data.frame in a ggtree object (adding "defmuts", "allmuts", "mouseover", "colour_var" columns, and a column for each mutation regex that is passed into append_interactivity_data). Of these, the only columns that were actually used when setting up the interactive ggtree/girafe object were "colour_var" and "mouseover" and the mutation regex columns were used when making the genotype-heatmap.

We removed the "defmuts" column because this isn't presented by the tooltip, and isn't used in any other downstream processing step.

The "allmuts" column was not kept in either. But for different reasons. It is used to create the mutation-regex columns that the mutation heatmap is made from. Rather than defining these columns in append_interactivity_data (since they aren't presented 'interactively' to the user) they are now defined by extract_genotype_data where they are used to define the heatmap matrix and are thrown away.

@russHyde
Copy link
Collaborator Author

russHyde commented Dec 7, 2022

After the changes:
image

@russHyde
Copy link
Collaborator Author

russHyde commented Dec 7, 2022

Before the changes:
image

@russHyde
Copy link
Collaborator Author

russHyde commented Dec 7, 2022

I think the tooltip looks a bit scruffy. There's a preceding newline (possibly due to a hidden kable header) and there is the data.frame "Column.NAME" dot-separation mangling going on. I can fix these to just say

---------         
Cluster ID  NNNNN
---------

or just

Cluster ID: NNNNN

Copy link

@jr-nicola jr-nicola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I'd probably go for the second option of having just:

Cluster ID: NNNNN

@russHyde
Copy link
Collaborator Author

Thanks @jr-nicola I'll modify the tooltip text

@russHyde
Copy link
Collaborator Author

Tooltip just displays "Cluster ID: NNNN" now:

image

@russHyde russHyde requested a review from jr-nicola December 15, 2022 14:56
Copy link

@jr-nicola jr-nicola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jr-nicola
Copy link

When I ran this updated version on the larger data set and tried to parse the tooltips to get the cluster ID as a numeric, I got a couple of examples where the label was:

Cluster ID: NA

I assume these were already in the previous version, and our parsing just didn't return an error the previous time?

@russHyde
Copy link
Collaborator Author

I've just reran the code here. Calling girafe(ggobj = tree, options = girafe_options) throws warnings (as it did above), but it no longer prints the girafe object to the Viewer panel in RStudio. Will investigate further.

Revert "ci: add bioconductor CI checks (biocthis::use_bioc_github_act…
@russHyde russHyde force-pushed the trim-tooltip branch 2 times, most recently from 3fac6ff to 3961cc4 Compare January 20, 2023 12:09
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