-
Notifications
You must be signed in to change notification settings - Fork 28
[python] Outgest additional X layers to AnnData #2119
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
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2119 +/- ##
==========================================
+ Coverage 67.05% 71.77% +4.72%
==========================================
Files 49 101 +52
Lines 2577 6920 +4343
Branches 211 211
==========================================
+ Hits 1728 4967 +3239
- Misses 747 1851 +1104
Partials 102 102
Flags with carried forward coverage won't be shown. Click here to find out more.
|
@aaronwolen @mojaveazure ping 🙏 |
1 similar comment
@aaronwolen @mojaveazure ping 🙏 |
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.
Limiting the user's export options to either all or none of the X layers may be too inflexible. Ideally, they should be able to specify any subset of the layers they want.
@aaronwolen thanks! To avoid length iteration cycles, please take a minute or two to simply offer a candidate method signature, and I will do what you ask Thank you in advance! |
c9b0417
to
de0f301
Compare
4fe35e2
to
73db536
Compare
I don't know why this is suddenly failing CI in files I did not touch (Python 3.7 only). I am actively debugging. |
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.
Thanks for the productive discussions. Tested it out and it works great.
I don't know why this is now failing Python 3.7 CI on tests which I did not touch. It passes tests for me on my laptop. I will iteratively debug on #2137. |
# ---------------------------------------------------------------- | ||
def to_anndata( | ||
experiment: Experiment, | ||
measurement_name: str, | ||
*, | ||
X_layer_name: Optional[str] = "data", | ||
extra_X_layer_names: Optional[ | ||
Union[Sequence[str], collections.abc.KeysView[str]] |
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.
This needs to be typing.KeysView
for 3.7 compatibility.
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.
Thank you @ihnorton !!!!!
71e2dd6
to
8824532
Compare
8824532
to
33e5679
Compare
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-release-1.7 release-1.7
# Navigate to the new working tree
cd .worktrees/backport-release-1.7
# Create a new branch
git switch --create backport-2119-to-release-1.7
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick --mainline 1 d9c4e07eb8e05a99218df3876244a2de025f5ae5
# Push it to GitHub
git push --set-upstream origin backport-2119-to-release-1.7
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-release-1.7 Then, create a pull request where the |
* [python] Outgest all X layers to AnnData * Unit-test coverage * typofix * doc-string and API update; impl on next commit, and unit-test cases after that * code mods to match API/docstrings; unit-test updates on next commit * updated unit-test cases * compatibility * ihnorton advice * merge
* [python] Outgest all X layers to AnnData * Unit-test coverage * typofix * doc-string and API update; impl on next commit, and unit-test cases after that * code mods to match API/docstrings; unit-test updates on next commit * updated unit-test cases * compatibility * ihnorton advice * merge
[sc-40032] |
This pull request has been linked to Shortcut Story #40032: Support exporting additional X layers to anndata. |
Issue and/or context: Issue #2079