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

[python] Ingest 2D uns string arrays (e.g. color labels) #1763

Merged
merged 5 commits into from
Oct 9, 2023
Merged

[python] Ingest 2D uns string arrays (e.g. color labels) #1763

merged 5 commits into from
Oct 9, 2023

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Oct 6, 2023 •
edited
Loading

Issue and/or context: #1764

Changes: As part of uns-outgest testing, I am also seeing we're not ingesting uns in all cases.

Namely, the code looks like this:
https://github.com/single-cell-data/TileDB-SOMA/blob/1.5.0rc1/apis/python/src/tiledbsoma/io/ingest.py#L2190-L2196

when AnnData inputs look like this:

>>> adata.uns.keys()
dict_keys(['age_colors', 'celltype_colors', 'hvg', 'leiden', 'louvain', 'louvain_colors', 'neighbors', 'pca', 'tissue_colors', 'umap'])

>>> adata.uns["age_colors"]
array([['#e1f3b2']], dtype=object)

>>> adata.uns["celltype_colors"]
array([['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080'],
       ['#808080']], dtype=object)

>>> adata.uns["louvain_colors"]
array([['#FFFF00'],
       ['#1CE6FF'],
       ['#FF34FF'],
       ['#FF4A46'],
       ['#A30059'],
       ['#FFDBE5'],
       ['#0000A6'],
       ['#8FB0FF'],
       ['#FF2F80'],
       ['#B903AA'],
       ['#A079BF'],
       ['#CC0744'],
       ['#C0B9B2'],
       ['#6F0062']], dtype=object)

>>> adata.uns["tissue_colors"]
array([['#e7cb94']], dtype=object)

Notes for the reviewer:

@johnkerl johnkerl changed the title [python] Ingest 2D uns string arrays (e.g. color labels) [python] Ingest 2D uns string arrays (e.g. color labels) [WIP] Oct 6, 2023
@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from 3555873 to 50c4a59 Compare October 6, 2023 17:29
@johnkerl johnkerl requested review from aaronwolen and nguyenv October 6, 2023 17:49
@johnkerl johnkerl marked this pull request as ready for review October 6, 2023 17:49
@johnkerl johnkerl changed the title [python] Ingest 2D uns string arrays (e.g. color labels) [WIP] [python] Ingest 2D uns string arrays (e.g. color labels) Oct 6, 2023
@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from 5a9103d to ec2f63e Compare October 6, 2023 18:00
@codecov-commenter
Copy link

codecov-commenter commented Oct 6, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files Coverage Δ
apis/python/src/tiledbsoma/io/ingest.py 87.56% <100.00%> (+0.13%) ⬆️

... and 75 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@johnkerl johnkerl requested a review from thetorpedodog October 6, 2023 21:18
johnkerl
johnkerl commented Oct 6, 2023
View reviewed changes
apis/python/setup.py Outdated
@@ -269,7 +269,7 @@ def run(self):
install_requires=[
# Needed for Python 3.7 which anndata 0.9 doesn't support but we do
"anndata < 0.9; python_version<'3.8'",
"anndata; python_version>='3.8'",
"anndata~=0.9.0; python_version>='3.8'", # XXX TEMP
Copy link
Member Author

Choose a reason for hiding this comment

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

anndata 0.10.0 just dropped and this is affecting ALL our Python PRs right now & I'll come up with a way to handle this -- got some good advice from @thetorpedodog in Slack ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Tracked in #1765

@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from ec2f63e to 6d0f456 Compare October 9, 2023 12:48
@johnkerl johnkerl changed the base branch from main to kerl/anndata-0-9-workaround October 9, 2023 12:49
@johnkerl
Copy link
Member Author

johnkerl commented Oct 9, 2023

This is stacked atop the blocking PR #1766 until that PR is merged, at which point this PR will be on main.

@johnkerl johnkerl force-pushed the kerl/anndata-0-9-workaround branch from 75764c2 to 1a95736 Compare October 9, 2023 12:51
@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from 6d0f456 to d62085f Compare October 9, 2023 12:52
@johnkerl johnkerl force-pushed the kerl/anndata-0-9-workaround branch from 1a95736 to 21ac225 Compare October 9, 2023 13:06
@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from d62085f to 150aa68 Compare October 9, 2023 13:07
@johnkerl johnkerl force-pushed the kerl/anndata-0-9-workaround branch from 21ac225 to 9d4c0a0 Compare October 9, 2023 13:11
@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from 150aa68 to f775b86 Compare October 9, 2023 13:14
Base automatically changed from kerl/anndata-0-9-workaround to main October 9, 2023 19:57
eddelbuettel
eddelbuettel approved these changes Oct 9, 2023
View reviewed changes
Copy link
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Looks clean to my (only moderately discerning for Python) eyes

@johnkerl johnkerl force-pushed the kerl/uns-2d-string-array-ingest branch from f775b86 to fee4da3 Compare October 9, 2023 20:04
thetorpedodog
thetorpedodog approved these changes Oct 9, 2023
View reviewed changes
apis/python/src/tiledbsoma/io/ingest.py Outdated Show resolved Hide resolved
nguyenv
nguyenv approved these changes Oct 9, 2023
View reviewed changes
Copy link
Member

@nguyenv nguyenv left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@johnkerl johnkerl merged commit 8d6b1c7 into main Oct 9, 2023
@johnkerl johnkerl deleted the kerl/uns-2d-string-array-ingest branch October 9, 2023 21:25
github-actions bot pushed a commit that referenced this pull request Oct 9, 2023
@johnkerl @github-actions
[python] Ingest 2D uns string arrays (e.g. color labels) (#1763)
e91a8c4 
* [python] Ingest 2D uns string arrays (e.g. color labels)

* unit-test case

* unit-test case

* comments

* code-review feedback
@github-actions github-actions bot mentioned this pull request Oct 9, 2023
Merged
johnkerl added a commit that referenced this pull request Oct 9, 2023
@github-actions @johnkerl
[python] Ingest 2D uns string arrays (e.g. color labels) (#1763) (#1773)
dfa5b99 
* [python] Ingest 2D uns string arrays (e.g. color labels)

* unit-test case

* unit-test case

* comments

* code-review feedback

Co-authored-by: John Kerl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thetorpedodog thetorpedodog thetorpedodog approved these changes

@nguyenv nguyenv nguyenv approved these changes

@eddelbuettel eddelbuettel eddelbuettel approved these changes

@aaronwolen aaronwolen Awaiting requested review from aaronwolen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@johnkerl @codecov-commenter @eddelbuettel @nguyenv @thetorpedodog