-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add gene annotations to anndata vars #23
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #23 +/- ##
==========================================
- Coverage 93.69% 92.72% -0.98%
==========================================
Files 6 7 +1
Lines 333 371 +38
==========================================
+ Hits 312 344 +32
- Misses 21 27 +6
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Outstanding issues:
|
Co-authored-by: Isaac Virshup <[email protected]>
def annotate_anndata( | ||
adata_var: pd.DataFrame, | ||
annotation_df: pd.DataFrame, | ||
on: str = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the variable names a bit confusing because in pandas syntax on
means that the column is present in both tables. Maybe we could name it left_on
and right_on
like in pd.merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed to annotation_on
and var_on
to match the other params
Looks good! I added a few minor comments :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, looks good.
One comment on the tutorial, can we make it so the AnnData
used the same ensembl version as we annotate with? Either by using a different ensembl version, or a different AnnData
.
@ivirshup wrt Ensembl version matching, the pbmc3k data was mapped with cellranger 1.1, but I am having a hard time matching this version to a Ensembl release. I'm gonna use the data from |
Closes #14