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

[python] Add test for to_anndata column_names #763

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

bkmartinjr
Copy link
Member

@bkmartinjr bkmartinjr commented Jan 19, 2023

Issue and/or context:

See #755 and chanzuckerberg/cellxgene-census#56

Changes:

Add a test to validate changes introduced in single-cell-data/SOMA#89

Notes for Reviewer:

Please review with single-cell-data/SOMA#89

IMPORTANT: This test will fail until tiledbsoma is upgraded to use the fix in somacore

@bkmartinjr bkmartinjr marked this pull request as ready for review January 19, 2023 20:16
@johnkerl johnkerl changed the title add test for to_anndata column_names [python] Add test for to_anndata column_names Jan 19, 2023
Copy link
Contributor

@thetorpedodog thetorpedodog left a comment

Choose a reason for hiding this comment

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

typo and a trivail* suggestion but looks great

* this typo made on accident but too funny to remove

"""
Verify that column_names is correctly handled in the various obs/var accessors.

Returned columsn should be the union of the columns specifically requested via
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Returned columsn should be the union of the columns specifically requested via
Returned columns should be the union of the columns specifically requested via

with soma_experiment.axis_query(
"RNA",
obs_query=soma.AxisQuery(
value_filter="label in [" + ",".join([f"'{i}'" for i in range(101)]) + "]"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you can pass a generator to .join rather than reifying it into a list:

Suggested change
value_filter="label in [" + ",".join([f"'{i}'" for i in range(101)]) + "]"
value_filter="label in [" + ",".join(f"'{i}'" for i in range(101]) + "]"

@bkmartinjr bkmartinjr merged commit b215cdb into main Jan 19, 2023
@bkmartinjr bkmartinjr deleted the bkmartinjr/755-query-colnames branch January 19, 2023 21:48
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.

2 participants