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.

Sign up for GitHub

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

Jump to bottom

Vdj plot - [merged] #79

Closed
grst opened this issue Apr 9, 2020 · 70 comments
Closed

Vdj plot - [merged] #79

grst opened this issue Apr 9, 2020 · 70 comments

Comments

@grst
Copy link
Collaborator

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 25, 2020, 19:57

Merges vdj_plot -> master

Added a much faster version of #24
Fixes #24.

Test cases need to be added yet.
@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 25, 2020, 19:59

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 25, 2020, 20:31

added 1 commit

  • 1067b5f - Temporary fix for size_column

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 25, 2020, 21:52

changed the description

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 12:34

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 15:32

added 5 commits

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 15:52

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 16:06

added 37 commits

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 16:10

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 26, 2020, 20:12

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 09:34

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 09:40

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 09:52

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 10:38

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:47

I would get rid of the for_cells parameter.
The same can be achieved by simply subsetting adata: ir.pl.vdj_usage(adata[for_cells, :], ...).

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:49

Maybe it would be more convenient to specify a column to normalize to instead of weights?
e.g. specify sample to normalize to sample size.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:50

ah.. just saw that you have the fraction_base column below.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:52

Can we harmonize that into a single argument?
E.g. Union[str, Sequence[float], None]: weight that takes either a sequence or a column name.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:59

same as above

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 10:59

also, same as above...

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 11:01

Just return the dataframe.
I do that for all other tools, and if someone really wants to have a dict the dataframe is easy to convert.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 11:09

This, actually, does modify adata inplace.

If this function just computes the size_column, you could just return the size column instead?

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 11:10

Added some comments in a first round of review: I mainly ask to remove a few parameters that I think are not necessary.
Looks pretty nice otherwise!

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:13

unmarked as a Work In Progress

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:17

I thought of it as a kind of shortcut. But we don't need it.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:24

I will call it fraction base then, because I think this way the name we used for other functions as well.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 12:25

it's only fraction by now in the other functions I think (and allows to specify True/False or a column name).

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:28

I would rather return a dataframe. This would also align better with how the other functions work.
Now I make a copy of obs and filter that.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:38

right, I modified accordingly

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 12:40

changed this line in version 14 of the diff

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 14:39

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 14:50

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 15:04

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 15:21

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 15:33

I was trying to implement the _normalize_counts approach. This gives me the size of the group instead of cell weight, however. Is this an intended behaviour? I will use 1/group size for cell weight then.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 19:00

What I meant is that do we have a scenario where we directly use the group sizes? Or would it make sense for the _normalize_counts function to return 1/group size (aka weights) directly...

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 19:10

added 1 commit

  • e918400 - Use weights for cell fractions

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 19:27

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 19:38

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 27, 2020, 19:54

I, indeed, use it like this:
https://gitlab.i-med.ac.at/icbi-lab/pipelines/singlecell_tcr/blob/master/scirpy/_tools/_group_abundance.py#L21

if it makes more sense to you, feel free to change that accordingly. Just make sure that you update all occurrences of _normalize_counts.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 20:51

added 1 commit

  • 4de7851 - Changed normalization to weights (1=n)

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 20:51

Ok, I changed it for now

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 27, 2020, 21:05

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 08:17

changed this line in version 29 of the diff

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 08:17

changed this line in version 29 of the diff

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 08:17

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 08:20

I removed the tool now entirely - it didn't do anything vdj_usage specific, but just add the weight column.
That's why I think it's better to call that utility function straight from the plot.

Apart from the question about the ribbon widths I just sent you on slack this is ready to merge now.

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 08:32

added 1 commit

  • e57c412 - Reformat docs, accept fig_kw argument.

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 28, 2020, 10:19

added 1 commit

  • 1f1b070 - Implementing full_combination switch

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 28, 2020, 10:32

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 28, 2020, 10:58

added 1 commit

  • 044297d - Added full combination switch

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 28, 2020, 11:12

added 1 commit

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @szabogtamas on Mar 28, 2020, 11:26

added 1 commit

  • 0f3b0c5 - Adding example for paired combinations

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:17

added 50 commits

Compare with previous version

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:18

enabled an automatic merge when the pipeline for 755616d succeeds

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:18

canceled the automatic merge

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:18

enabled an automatic merge when the pipeline for 755616d succeeds

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:24

merged

@grst
Copy link
Collaborator Author

grst commented Apr 9, 2020

In GitLab by @grst on Mar 28, 2020, 12:24

mentioned in commit 4b76f27

@grst grst closed this as completed Apr 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@grst