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] Support new set-membership query condition #1756

Merged
merged 6 commits into from
Oct 12, 2023

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Oct 4, 2023

Issue and/or context: This is an adaptation of PR TileDB-Inc/TileDB-Py#1837 for issue #1742

@johnkerl johnkerl changed the title [python] Set-membership API [WIP] [python] Support new set-membership query condition [WIP] Oct 4, 2023
@johnkerl
Copy link
Member Author

johnkerl commented Oct 4, 2023

The unit-test case is failing because of this, needing to be fixed:

Before:

>>> uri = 'tiledb://johnkerl-tiledb/oron-var'
>>> sdf = tiledbsoma.DataFrame.open(uri)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'tiledbsoma' is not defined
>>> import tiledbsoma
>>> sdf = tiledbsoma.DataFrame.open(uri)
>>> sdf.read(
...     column_names=['feature_name'],
...     value_filter='feature_id in ["ENSG00000148584"]',
... ).concat().to_pandas()
  feature_name
0         A1CF

After:

>>> uri = 'tiledb://johnkerl-tiledb/oron-var'
>>> sdf = tiledbsoma.DataFrame.open(uri)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'tiledbsoma' is not defined
>>> import tiledbsoma
>>> sdf = tiledbsoma.DataFrame.open(uri)
>>> sdf.read(
...     column_names=['feature_name'],
...     value_filter='feature_id in ["ENSG00000148584"]',
... ).concat().to_pandas()
  feature_name       feature_id
0         A1CF  ENSG00000148584

That is to say, we need to preserve the feature that the queried-for attribute name gets included in the list of output attrs.

See #755 for reference.

@codecov-commenter
Copy link

codecov-commenter commented Oct 4, 2023

Codecov Report

All modified lines are covered by tests ✅

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

see 36 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch 2 times, most recently from 40ecade to 0551b17 Compare October 5, 2023 14:19
@johnkerl johnkerl changed the title [python] Support new set-membership query condition [WIP] [python] Support new set-membership query condition Oct 5, 2023
@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch from c1d7614 to a703226 Compare October 5, 2023 14:29
@johnkerl johnkerl marked this pull request as ready for review October 5, 2023 14:40
@johnkerl
Copy link
Member Author

johnkerl commented Oct 5, 2023

(BTW the aforementioned unit-test case has since been fixed)

@johnkerl johnkerl requested a review from nguyenv October 5, 2023 16:20
@johnkerl
Copy link
Member Author

johnkerl commented Oct 6, 2023

CI fail

E   ImportError: cannot import name 'SparseDataset' from 'anndata._core.sparse_dataset' (/opt/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/anndata/_core/sparse_dataset.py)

as AnnData 0.10.0 dropped today:
https://anndata.readthedocs.io/en/latest/#version-0-10

@johnkerl
Copy link
Member Author

johnkerl commented Oct 6, 2023

Before:

anndata._core.sparse_dataset.SparseDataset

After:

anndata._core.sparse_dataset.CSRDataset
anndata._core.sparse_dataset.CSRDataset

@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch 2 times, most recently from 5c01308 to bffe837 Compare October 9, 2023 12:50
@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 changed the base branch from main to kerl/anndata-0-9-workaround October 9, 2023 12:50
@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch from bffe837 to 04d25fe Compare October 9, 2023 12:51
@johnkerl johnkerl force-pushed the kerl/anndata-0-9-workaround branch 2 times, most recently from 1a95736 to 21ac225 Compare October 9, 2023 13:06
@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch from 04d25fe to 65024c0 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/python-set-membership-api branch 2 times, most recently from 9b469a8 to 30cf2d6 Compare October 9, 2023 13:13
Base automatically changed from kerl/anndata-0-9-workaround to main October 9, 2023 19:57
@johnkerl
Copy link
Member Author

Update: this PR was a very simple copy of TileDB-Inc/TileDB-Py#1837

Along the way we ran into the following, which were fixed:

So this should now be a very simple approve.

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.

one minor thing that you might want to address before submitting but not a showstopper.

apis/python/src/tiledbsoma/_query_condition.py Outdated Show resolved Hide resolved
@johnkerl johnkerl force-pushed the kerl/python-set-membership-api branch from 42a0cd8 to 07cb4ca Compare October 12, 2023 20:54
@johnkerl johnkerl merged commit e63989a into main Oct 12, 2023
9 checks passed
@johnkerl johnkerl deleted the kerl/python-set-membership-api branch October 12, 2023 21:33
github-actions bot pushed a commit that referenced this pull request Oct 12, 2023
* [python] Support new set-membership query condition

* unit-test coverage for not-in

* code-review feedback

* lint

* attempt to work around anndata 0.10.0 [WIP]

* undo anndata 0.10.0 attempted workaround; code-review feedback
johnkerl added a commit that referenced this pull request Oct 12, 2023
* [python] Support new set-membership query condition

* unit-test coverage for not-in

* code-review feedback

* lint

* attempt to work around anndata 0.10.0 [WIP]

* undo anndata 0.10.0 attempted workaround; 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants