[python] Dataframe read path by nguyenv · Pull Request #1793 · single-cell-data/TileDB-SOMA

nguyenv · 2023-10-16T21:50:05Z

Issue and/or context:

Running Changes:

When opening a DataFrame in read-mode, use DataFrameWrapper which wraps around clib.SOMADataFrame. Otherwise, DataFrame should use the already existing write-path with ArrayWrapper which wraps around a TileDB-Py Array
Corrections to C++ unit tests where SOMADataFrame needs to use sparse array
SOMADataFrame::count needs to use nnz
C++ API create needs to set soma_object_type and exists needs to check soma_object_type
Get full nonempty domains for SOMADataFrame
Add getters and methods to DataFrameWrapper and ArrayWrapper (i.e nonempty domain, attr names, dim names, etc)
Support query conditions for SOMADataFrame
_query_condition.py no longer has dependency on TileDB-Py and uses Pyarrow schema instead of TileDB ArraySchema and SOMAError instead of TileDBError
Reorganize Pybind11 code to place util functions and PyQueryCondition (note: should refactor QueryCondition to completely remove PyQueryCondition usage) into common.h
SOMAArray now resides in its own soma_array.cc file
pytiledbsoma.cc is now the top-level file that contains stats bindings and loads modules from query_condition.cc, soma_array.cc, soma_dataframe.cc
Support enumerations / dictionaries for SOMADataFrame
Support all enumeration values in ArrowAdapter::to_arrow
read Pybind11 function now uses correct Arrow schema to convert dictionary arrays when importing from C to Python
SOMAObject::schema returns ArrowSchema which calls from SOMAArray::arrow_schema
Create C++ factory unique_ptr<SOMAObject> SOMAObject::open and bind in Pybind11 as clib.SOMAObject.open which returns the correct Python SOMA class
Fix for [python] Errors raised as RuntimeError should raise SOMAError #783

codecov-commenter · 2023-10-16T22:03:34Z

Codecov Report

Merging #1793 (53cca7e) into main (08931d3) will decrease coverage by 1.55%.
The diff coverage is 66.01%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1793      +/-   ##
==========================================
- Coverage   77.59%   76.05%   -1.55%     
==========================================
  Files          87      136      +49     
  Lines        7784    10552    +2768     
  Branches        0      206     +206     
==========================================
+ Hits         6040     8025    +1985     
- Misses       1744     2433     +689     
- Partials        0       94      +94

Flag	Coverage Δ
libtiledbsoma	`67.69% <26.31%> (?)`
python	`91.40% <84.36%> (+2.31%)`	⬆️
r	`67.72% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
python_api	`91.40% <84.36%> (+2.31%)`	⬆️
libtiledbsoma	`50.61% <14.28%> (∅)`

thetorpedodog

(the usual disclaimer that I’m mostly looking at the Python side for style etc. and didn’t look at the C++ code in detail)

thetorpedodog · 2023-11-07T20:24:23Z

(the usual disclaimer that I’m mostly looking at the Python side for style etc. and didn’t look at the C++ code in detail)

I should add that I like the way the open-and-select-the-right-wrapper thing works now. It’s very well structured and understandable.

johnkerl

Nothing huge to sugggest -- some spelling changes etc

* Move `PyQueryCondition` Into `common.h` * Use Pyarrow Schema instead of TileDB ArraySchema * Remove TileDB-Py dependency * No longer requires attr-to-enum mapping passed for dictionaries as this can be checked in Pyarrow Schema now

* Eventually the `arrow_schema` calls should replace `schema` but quite a few things still depend on the TileDB ArraySchema so this is going to be temporarily punted for now

* The R API should be refactored soon to not rely on tiledb::ArraySchema

…me.open`

nguyenv · 2024-01-19T21:10:28Z

@beroy I have now merged your changes into my PR. The only change I did was move your reindexer unit tests to reside within the apis/python/tests directory. This is a part of changes within this PR where I've moved all Python unit tests from libtilesoma/tests into the Python API directory as to not have them split between two directories. Because this PR is already large enough as is, I have decided to leave the reindexer bindings within pytiledbsoma.cc for now but will be opening a subsequent PR where this gets separated out into its own reindexer.cc.

…SOMA#1793

nguyenv force-pushed the viviannguyen/python-read-path branch from 7ee0b85 to 8992661 Compare October 17, 2023 15:44

johnkerl changed the title ~~[python] Read Path~~ [python] Read path Oct 18, 2023

nguyenv force-pushed the viviannguyen/python-read-path branch from c0d02b0 to 4e63bc4 Compare October 19, 2023 00:36

nguyenv mentioned this pull request Oct 20, 2023

[python/c++] Expose uri, result_order, and column_names for SOMAArray #1814

Merged

nguyenv force-pushed the viviannguyen/python-read-path branch from 7db68e1 to 2c8fa29 Compare October 20, 2023 21:32

nguyenv changed the title ~~[python] Read path~~ [python] Dataframe read path Oct 20, 2023

nguyenv marked this pull request as ready for review October 20, 2023 23:16

nguyenv requested review from beroy, eddelbuettel, gspowley, ihnorton, johnkerl and thetorpedodog October 20, 2023 23:17

This was referenced Oct 23, 2023

[python] NDArray read path #1817

Closed

[python] Sync TileDB-Py and TileDB-SOMA Python query-condition code #1667

Closed

thetorpedodog reviewed Oct 27, 2023

View reviewed changes

nguyenv mentioned this pull request Nov 2, 2023

[c++/python] Expanded enumeration support in ArrowAdapter::to_arrow #1848

Merged

nguyenv force-pushed the viviannguyen/python-read-path branch 5 times, most recently from 4477714 to 4306121 Compare November 7, 2023 16:26

thetorpedodog reviewed Nov 7, 2023

View reviewed changes

nguyenv requested a review from thetorpedodog November 9, 2023 01:45

nguyenv force-pushed the viviannguyen/python-read-path branch 2 times, most recently from d5f4dc5 to 8f7951e Compare November 9, 2023 22:29

nguyenv mentioned this pull request Nov 14, 2023

[python] Normalize unit-test exception coverage #1909

Merged

johnkerl suggested changes Nov 14, 2023

View reviewed changes

Vivian Nguyen added 22 commits January 19, 2024 12:59

Add Dictionary Support For ArraySchema -> ArrowSchema

defb5bc

Refactor QueryCondition

72589f8

* Move `PyQueryCondition` Into `common.h` * Use Pyarrow Schema instead of TileDB ArraySchema * Remove TileDB-Py dependency * No longer requires attr-to-enum mapping passed for dictionaries as this can be checked in Pyarrow Schema now

SOMADataFrame --> DataframeWrapper

f25193b

Add Pybind11 Common Code

8a87d35

Reduce Usage of TileDB ArraySchema in Favor in Pyarrow Schema

b920697

Support All Enumeration Value Types in to_arrow

fc449e1

Do TileDB ArraySchema to ArrowSchema Converstion in C++

0b94a02

* Eventually the `arrow_schema` calls should replace `schema` but quite a few things still depend on the TileDB ArraySchema so this is going to be temporarily punted for now

Correct exists Method For macos

b1156b9

Add domain for SOMADataFrame

4fdffd8

TileDB ArraySchema to ArrowSchema Should Not Hardcode String Dict

c52e99a

Re-add SOMAArray::schema -> TileDB ArraySchema

9e4103f

* The R API should be refactored soon to not rely on tiledb::ArraySchema

Remove #include <tiledb/tiledb> from Array derived SOMAObjects

81480fa

Clean Up NED

ca37d67

Use DTypeLike instead of np.dtype[Any]

ba7aaad

Create Factory in Pybind11, Bind SOMAError

ff3e4b8

Add soma_object.cc files

de1ffee

Use Explcit Size For String SOMA Type Metadata to Correct Error on macOS

960b883

Replace with TPY_ERROR_LOC where necessary

ef577fc

Correct MacOS Exception Handling for Getting SOMA Object Type

8aacce4

Fix segfault by not spawning multiple ManagedQueries per SOMAArray

f0a96a0

Correct QueryCondition, Rename tiledb_schema, Remove _set_internal

19bd2e2

Use tiledb_schema to get ArraySchema

9bb751d

nguyenv force-pushed the viviannguyen/python-read-path branch from 7c4b511 to 03e7a61 Compare January 19, 2024 19:17

Move all Python tests to same directory, Require KWs for `SOMADataFra…

53cca7e

…me.open`

nguyenv force-pushed the viviannguyen/python-read-path branch from 03e7a61 to 53cca7e Compare January 19, 2024 19:47

nguyenv merged commit 44bfad4 into main Jan 19, 2024

nguyenv deleted the viviannguyen/python-read-path branch January 19, 2024 21:14

jdblischak added a commit to TileDB-Inc/centralized-tiledb-nightlies that referenced this pull request Jan 25, 2024

The tiledbsoma-py tests were consolidated in single-cell-data/TileDB-…

954206e

…SOMA#1793

nguyenv mentioned this pull request Feb 6, 2024

[python] Reorganize Reindexer bindings into separate file #2110

Merged

Conversation

nguyenv commented Oct 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Oct 16, 2023 • edited by codecov Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thetorpedodog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thetorpedodog commented Nov 7, 2023

Uh oh!

johnkerl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nguyenv commented Jan 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nguyenv commented Oct 16, 2023 •

edited

Loading

codecov-commenter commented Oct 16, 2023 •

edited by codecov Bot

Loading