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

[r] Use the new tiledb-r release in CI #1663

Merged
merged 8 commits into from
Sep 15, 2023
Merged

Conversation

eddelbuettel
Copy link
Contributor

@eddelbuettel eddelbuettel commented Sep 7, 2023

Issue and/or context:

We need the Enumerations-aware tiledb-r in CI. This adds it.

Changes:

(Temporarily) adds two new segments for macOS and linux to r-ci.yaml

Notes for Reviewer:

SC 33875

#866

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #33875: [r] Temporary switch to rc builds in CI.

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.

🚢

LGTM as long as CI passes! :)

@eddelbuettel
Copy link
Contributor Author

🤞

@eddelbuettel eddelbuettel force-pushed the de/sc-33875/use_rc_in_ci branch from 1d4d64a to 133f59c Compare September 8, 2023 20:53
@codecov-commenter
Copy link

codecov-commenter commented Sep 8, 2023

Codecov Report

Patch has no changes to coverable lines.

❗ Current head a445cd5 differs from pull request most recent head af37773. Consider uploading reports for the commit af37773 to get more accurate results

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

📢 Thoughts on this report? Let us know!.

@johnkerl
Copy link
Member

@eddelbuettel we can close this now I believe

@eddelbuettel
Copy link
Contributor Author

@johnkerl I fear not as eg r-ci.yml goes to r-universe because what it needs is not available elsewhere and may not be for a day.

@johnkerl
Copy link
Member

@eddelbuettel OK.

We can't tag 1.5.0 until all 2.17.0-rc0 dependencies are gone, but, if this PR helps soothe concerns until then, all for the good.

johnkerl added a commit that referenced this pull request Sep 14, 2023
* [c++] Support `Enumeration` in C++ Codebase

* Addition of `SOMAArray::get_enum` and `SOMAArray::get_enum_label_on_attr`
* Attach an enumeration/dictionary to the `ColumnBuffer` if applicable;
  this is used when converting from `ArrayBuffers` to Arrow Tables in
  the Python and R APIs

* Add `get_attr_to_enum_mapping` Function

* WIP fix bug where attr name was passed instead of enum name

* Add Unit Tests for Enumeration in C++

* `to_varlen_buffers` Returns `std::string`

* Prior to TileDB-Inc/TileDB#4272, the SOMA unit
tests were erroneously writing a byte vector for string dimensions which
maps to `TILEDB_BLOB` rather than `TILEDB_STRING_ASCII`

* Update SOMA Array get_metadata Signatures

* Depend on 2.17.0-rc0

* resolve an incomplete merge with #1559

* [r] Update tiledb-r to RC (borrowed from #1663, #1665)

* [r] Undo brown-bag typo in helper script

* 2.17.0

---------

Co-authored-by: John Kerl <[email protected]>
Co-authored-by: Dirk Eddelbuettel <[email protected]>
@johnkerl
Copy link
Member

@eddelbuettel I did git merge main on this just now & there's not much left -- we are converging! :)

@eddelbuettel eddelbuettel changed the title [r] Use the tiledb-r RC in CI [r] Use the new tiledb-r release in CI Sep 15, 2023
@johnkerl johnkerl merged commit 28213a3 into main Sep 15, 2023
@johnkerl johnkerl deleted the de/sc-33875/use_rc_in_ci branch September 15, 2023 15:39
ihnorton pushed a commit that referenced this pull request Sep 15, 2023
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.
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