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

[c++] Performant DataFrame.shape #2916

Merged
merged 8 commits into from
Sep 3, 2024
Merged

[c++] Performant DataFrame.shape #2916

merged 8 commits into from
Sep 3, 2024

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Aug 18, 2024

Issue and/or context: This follows on #2944 for issue #2407 / [sc-51048]. Here we implement shape for SOMA DataFrame, including its variant-indexed flavors.

Changes:

While SOMASparseNDArray and SOMADenseNDArray must always and only have int64 dims within the SOMA data model, SOMADataFrame is different. The default behavior -- which almost everyone uses -- has a single soma_joinid dim which is indeed of type int64. (Also note that exp.obs.shape does exist within TileDB-SOMA-Py, and people do call it.) However, the spec only requires that soma_joinid exist as a dim or an attr: it can be a dim along with others, or it can not be a dim at all. Here we do the right thing, without and with current-domain support, to produce the user-expected exp.obs.shape.

Notes for Reviewer:

Copy link

codecov bot commented Aug 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.02%. Comparing base (ded1e42) to head (b2ef153).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2916      +/-   ##
==========================================
+ Coverage   89.87%   90.02%   +0.15%     
==========================================
  Files          38       38              
  Lines        3999     3999              
==========================================
+ Hits         3594     3600       +6     
+ Misses        405      399       -6     
Flag Coverage Δ
python 90.02% <ø> (+0.15%) ⬆️

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

Components Coverage Δ
python_api 90.02% <ø> (+0.15%) ⬆️
libtiledbsoma ∅ <ø> (∅)

@johnkerl johnkerl force-pushed the kerl/arrow-util-current-domain-optional branch 3 times, most recently from a51f6bd to daecd1b Compare August 19, 2024 02:41
@johnkerl johnkerl force-pushed the kerl/sdf-shape branch 2 times, most recently from 21155f6 to 696ab50 Compare August 19, 2024 03:05
@johnkerl johnkerl force-pushed the kerl/arrow-util-current-domain-optional branch 2 times, most recently from a903035 to 41c6d96 Compare August 20, 2024 14:06
@johnkerl johnkerl force-pushed the kerl/arrow-util-current-domain-optional branch from 41c6d96 to 1db05d7 Compare August 20, 2024 14:16
@johnkerl johnkerl force-pushed the kerl/sdf-shape branch 2 times, most recently from 0831bdb to 8c5b2af Compare August 20, 2024 20:00
@johnkerl johnkerl force-pushed the kerl/arrow-util-current-domain-optional branch from b58dba1 to ee61b8e Compare August 27, 2024 21:55
Base automatically changed from kerl/arrow-util-current-domain-optional to main August 27, 2024 23:01
@johnkerl johnkerl changed the base branch from main to kerl/cpp-variant-indexed-dataframes August 30, 2024 21:17
@johnkerl johnkerl requested a review from jp-dark August 30, 2024 21:56
@johnkerl johnkerl changed the title [c++] Performant DataFrame.shape [WIP] [c++] Performant DataFrame.shape Aug 30, 2024
@johnkerl johnkerl marked this pull request as ready for review August 30, 2024 21:58
libtiledbsoma/src/soma/soma_array.h Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_array.h Outdated Show resolved Hide resolved
Base automatically changed from kerl/cpp-variant-indexed-dataframes to main September 3, 2024 17:42
@johnkerl johnkerl merged commit f5ae258 into main Sep 3, 2024
15 checks passed
@johnkerl johnkerl deleted the kerl/sdf-shape branch September 3, 2024 19:21
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