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++] Merges for #1559 #1684

Merged
merged 3 commits into from
Sep 15, 2023
Merged

Conversation

johnkerl
Copy link
Member

@johnkerl johnkerl commented Sep 15, 2023

Addresses issues found on Python unit test running on an R/C++ PR with @ihnorton and @nguyenv

@johnkerl
Copy link
Member Author

Merging into #1559

Copy link
Contributor

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

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

Well done

@johnkerl
Copy link
Member Author

I'll let CI run there

@johnkerl johnkerl merged commit 0d7cb02 into de/sc-32235/enumeration_support Sep 15, 2023
@johnkerl johnkerl deleted the kerl/ihn-bugfix branch September 15, 2023 21:31
ihnorton pushed a commit that referenced this pull request Sep 15, 2023
* ihn/bugfix

* unit-test update

* lint
johnkerl added a commit that referenced this pull request Sep 15, 2023
* **Issue and/or context:**

As described in #1558 and #866, adding enumeration support is desirable once we have TileDB Embedded 2.17 available

**Changes:**

This PR supports reading of columns with enumerations (aka dictionaries aka factor variable) directly via Arrow. Preliminary write support is also available (but still goes through the `tiledb` R package for writes).

**Notes for Reviewer:**

~This PR is now work-in-progress and not ready for a merge while we await TileDB 2.17.~  The branch and PR are ready but should only be merged once prequisites are been merged.  It likely needs #1519 (C++ side) and #1663 (CI support).

CI is turned off as the TileDB default build is still without support for enumerations.

* **Issue and/or context:**

This PR adds support for return Arrow tables with dictionaries that can include `ordered` enumerations.

**Changes:**

Given #1559 which it depends upon, a very small change to just three files in `libtiledbsoma`.

This should become clearer once the dependent PR is merged and can be rebased.

**Notes for Reviewer:**

[SC 34073](https://app.shortcut.com/tiledb-inc/story/34073/c-add-ordered-support-to-arrow-export)

* **Issue and/or context:**

This PR extends the `schema()` function to return an Arrow schema with enumerations including `ordered`.

**Changes:**

Given #1559 which it depends upon, a very small change to just one file.

This should become clearer once the dependent PR is merged and can be rebased.

**Notes for Reviewer:**

[SC 34074](https://app.shortcut.com/tiledb-inc/story/34074/c-add-ordered-support-to-arrow-export)

* [c++] Test fixes for #1559 (#1684)

* ihn/bugfix

* unit-test update

* lint

---------

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.

3 participants