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++] Column abstraction: SOMAGeometryColumn, part 3 #3427

Merged
merged 25 commits into from
Jan 8, 2025

Conversation

XanthosXanthopoulos
Copy link
Collaborator

This PR introduces the SOMAGeometryColumn concrete class that implement a spatial index WKB (Well-Known Binary) column. A set of internal TileDB dimensions are used to provide the index mechanics as well as a TileDB Attribute to hold the geometry data encoded in a WKB blob.

@XanthosXanthopoulos XanthosXanthopoulos changed the title SOMAGeometryColumn, part 3 [c++] Column abstraction: SOMAGeometryColumn, part 3 Dec 12, 2024
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

@XanthosXanthopoulos thanks for working on this! The split-up makes this PR much easier to review.

libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/tiledbsoma/tiledbsoma Show resolved Hide resolved
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-geometry-column branch from fc53e6e to 497224a Compare December 13, 2024 15:25
@jp-dark jp-dark self-requested a review December 23, 2024 16:10
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-attribute branch 2 times, most recently from 73865cb to 5bcd88d Compare January 2, 2025 11:36
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-geometry-column branch from 497224a to e02f78e Compare January 2, 2025 13:48
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-attribute branch 2 times, most recently from b0a1976 to 3faefd9 Compare January 2, 2025 20:56
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-geometry-column branch from e02f78e to 6f4d6a2 Compare January 3, 2025 14:41
Base automatically changed from xan/sc-59427/soma-attribute to main January 3, 2025 16:08
@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-geometry-column branch from 6f4d6a2 to 8daf17e Compare January 7, 2025 16:39
Copy link

codecov bot commented Jan 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.30%. Comparing base (eabdb30) to head (a426c7a).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3427      +/-   ##
==========================================
+ Coverage   86.25%   86.30%   +0.04%     
==========================================
  Files          55       55              
  Lines        6359     6359              
==========================================
+ Hits         5485     5488       +3     
+ Misses        874      871       -3     
Flag Coverage Δ
python 86.30% <ø> (+0.04%) ⬆️

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

Components Coverage Δ
python_api 86.30% <ø> (+0.04%) ⬆️
libtiledbsoma ∅ <ø> (∅)

@johnkerl johnkerl self-requested a review January 8, 2025 16:02
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
libtiledbsoma/src/soma/soma_geometry_column.cc Outdated Show resolved Hide resolved
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

Saying either 'core domain', 'core current domain', 'soma domain', or 'soma maxdomain' is absoutely crucial for avoiding confusion. Saying 'domain' might mean any one of these four, and so it ambiguous. I requested these changes the last time; I'm still requesting them this time.

I know the names are confusing and that's why we must be absolutely vigilant to help out our co-workers, in the present and in the future, by being very clear.

@XanthosXanthopoulos XanthosXanthopoulos force-pushed the xan/sc-59427/soma-geometry-column branch from 8daf17e to a426c7a Compare January 8, 2025 19:04
Copy link
Member

@johnkerl johnkerl left a comment

Choose a reason for hiding this comment

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

Thank you @XanthosXanthopoulos !

@XanthosXanthopoulos XanthosXanthopoulos merged commit 824f648 into main Jan 8, 2025
26 checks passed
@XanthosXanthopoulos XanthosXanthopoulos deleted the xan/sc-59427/soma-geometry-column branch January 8, 2025 19:38
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.

[c++] Add an abstraction layer between SOMA columns and TileDB dimensions and attributes
3 participants