GH-36363: [MATLAB] Create proxy classes for the DataType class hierarchy #36419

sgilmore10 · 2023-06-30T19:36:20Z

Rationale for this change

In the original pull request in which we added the MATLAB arrow.type.<Type>Type classes (e.g. arrow.type.Float32Type), we did implement these classes as proxies. At the time, we weren't sure if it would be advantageous to implement the type classes as proxies, but now realize it will be for composite data structures, i.e. Schema, StructArray, ListArray.

What changes are included in this PR?

All classes within the arrow.type.Type class hierarchy are implemented as proxies.

Are these changes tested?

Yes, we had existing tests for these classes.

Are there any user-facing changes?

No.

Future Directions

In a followup PR request, we plan on integrating the proxy type classes and the array classes so that they share the same underlying C++ arrow::DataType object. We thought doing so in this change would be too much code churn.

Notes

Thank you @kevingurney for the help!

Closes: [MATLAB] Create proxy classes for the DataType class hierarchy #36363

kevingurney · 2023-07-07T17:07:17Z

Added myself as a reviewer per #36549.

kevingurney

+1

matlab/src/cpp/arrow/matlab/type/proxy/type.h

matlab/src/cpp/arrow/matlab/type/proxy/type.cc

matlab/src/cpp/arrow/matlab/proxy/factory.cc

matlab/src/cpp/arrow/matlab/type/proxy/type.cc

matlab/src/cpp/arrow/matlab/type/proxy/primitive_ctype.h

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

…ay` subclasses from existing proxy ids (#36731) ### Rationale for this change Now that the issue #36363 is closed via PR #36419, we can initialize the `Type` property of `arrow.array.Array` subclasses from existing proxy ids. Currently, we create a new proxy `Type` object whose underlying `arrow::DataType` are semantically equal to - but not the same as - the `arrow::DataType` owned by the Array proxy. It would be preferable if the `Type` and `Array` proxy classes refer to the same `arrow::DataType` object (i.e. the same object on the heap). ### What changes are included in this PR? 1. Upgraded `libmexclass` to commit [d04f88d](mathworks/libmexclass@d04f88d). In this commit, we added a static "make-like" function to `Proxy` called `create`. 2. Modified the constructors of all `Type` objects to expect a single `Proxy` object as input. This is a breaking change and means clients are no longer expected to build `Type` objects via their constructors. Instead, we introduced standalone functions that clients can use to construct `Type` objects, i.e. `arrow.type.int8`, `arrow.type.string`, `arrow.type.timestamp`, etc. These functions deal with creating the `Proxy` objects to pass to the `Type` constructors. Below is an example of the new workflow for creating `Type` objects. ```matlab >> timestampType = arrow.type.timestamp(TimeUnit="second", TimeZone="America/New_York") timestampType = TimestampType with properties: ID: Timestamp ``` NOTE: We plan on enhancing the display to show the `TimeUnit` and `TimeZone` properties. 3. Made `Type` a [dependent](https://www.mathworks.com/help/matlab/matlab_oop/access-methods-for-dependent-properties.html) property on `arrow.array.Array`. The `get.Type` method constructs a `Type` object on demand by making a proxy that wraps the same `arrow::DataType` object stored within the `arrow::Array`. ### Are these changes tested? Yes, updated existing tests. ### Are there any user-facing changes? Yes, we added new standalone functions for creating `Type` objects. Below is a table mapping standalone functions to the `Type` object they output: | Standalone Function | Output Type Object | |----------------------|---------------------| |`arrow.type.boolean`| `arrow.type.BooleanType`| |`arrow.type.int8`| `arrow.type.Int8Type`| |`arrow.type.int16`| `arrow.type.Int16Type`| |`arrow.type.int32`| `arrow.type.Int32Type`| |`arrow.type.int64`| `arrow.type.Int64Type`| |`arrow.type.uint8`| `arrow.type.UInt8Type`| |`arrow.type.uint16`| `arrow.type.UInt16Type`| |`arrow.type.uint32`| `arrow.type.UInt32Type`| |`arrow.type.uint64`| `arrow.type.UInt64Type`| |`arrow.type.string`| `arrow.type.StringType`| |`arrow.type.timestamp`| `arrow.type.TimestampType`| ### Notes Thanks @ kevingurney for the advice! * Closes: #36652 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>

…hierarchy (apache#36419) ### Rationale for this change In the original pull request in which we added the MATLAB `arrow.type.<Type>Type` classes (e.g. `arrow.type.Float32Type`), we did implement these classes as proxies. At the time, we weren't sure if it would be advantageous to implement the type classes as proxies, but now realize it will be for composite data structures, i.e. `Schema`, `StructArray`, `ListArray`. ### What changes are included in this PR? 1. All classes within the `arrow.type.Type` class hierarchy are implemented as proxies. ### Are these changes tested? Yes, we had existing tests for these classes. ### Are there any user-facing changes? No. ### Future Directions 1. In a followup PR request, we plan on integrating the proxy type classes and the array classes so that they share the same underlying C++` arrow::DataType` object. We thought doing so in this change would be too much code churn. ### Notes Thank you @ kevingurney for the help! * Closes: apache#36363 Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com> Co-authored-by: sgilmore10 <74676073+sgilmore10@users.noreply.github.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>

…ay.Array` subclasses from existing proxy ids (apache#36731) ### Rationale for this change Now that the issue apache#36363 is closed via PR apache#36419, we can initialize the `Type` property of `arrow.array.Array` subclasses from existing proxy ids. Currently, we create a new proxy `Type` object whose underlying `arrow::DataType` are semantically equal to - but not the same as - the `arrow::DataType` owned by the Array proxy. It would be preferable if the `Type` and `Array` proxy classes refer to the same `arrow::DataType` object (i.e. the same object on the heap). ### What changes are included in this PR? 1. Upgraded `libmexclass` to commit [d04f88d](mathworks/libmexclass@d04f88d). In this commit, we added a static "make-like" function to `Proxy` called `create`. 2. Modified the constructors of all `Type` objects to expect a single `Proxy` object as input. This is a breaking change and means clients are no longer expected to build `Type` objects via their constructors. Instead, we introduced standalone functions that clients can use to construct `Type` objects, i.e. `arrow.type.int8`, `arrow.type.string`, `arrow.type.timestamp`, etc. These functions deal with creating the `Proxy` objects to pass to the `Type` constructors. Below is an example of the new workflow for creating `Type` objects. ```matlab >> timestampType = arrow.type.timestamp(TimeUnit="second", TimeZone="America/New_York") timestampType = TimestampType with properties: ID: Timestamp ``` NOTE: We plan on enhancing the display to show the `TimeUnit` and `TimeZone` properties. 3. Made `Type` a [dependent](https://www.mathworks.com/help/matlab/matlab_oop/access-methods-for-dependent-properties.html) property on `arrow.array.Array`. The `get.Type` method constructs a `Type` object on demand by making a proxy that wraps the same `arrow::DataType` object stored within the `arrow::Array`. ### Are these changes tested? Yes, updated existing tests. ### Are there any user-facing changes? Yes, we added new standalone functions for creating `Type` objects. Below is a table mapping standalone functions to the `Type` object they output: | Standalone Function | Output Type Object | |----------------------|---------------------| |`arrow.type.boolean`| `arrow.type.BooleanType`| |`arrow.type.int8`| `arrow.type.Int8Type`| |`arrow.type.int16`| `arrow.type.Int16Type`| |`arrow.type.int32`| `arrow.type.Int32Type`| |`arrow.type.int64`| `arrow.type.Int64Type`| |`arrow.type.uint8`| `arrow.type.UInt8Type`| |`arrow.type.uint16`| `arrow.type.UInt16Type`| |`arrow.type.uint32`| `arrow.type.UInt32Type`| |`arrow.type.uint64`| `arrow.type.UInt64Type`| |`arrow.type.string`| `arrow.type.StringType`| |`arrow.type.timestamp`| `arrow.type.TimestampType`| ### Notes Thanks @ kevingurney for the advice! * Closes: apache#36652 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>

conbench-apache-arrow · 2023-07-22T11:55:13Z

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 6baf6a7.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

…hierarchy (apache#36419) ### Rationale for this change In the original pull request in which we added the MATLAB `arrow.type.<Type>Type` classes (e.g. `arrow.type.Float32Type`), we did implement these classes as proxies. At the time, we weren't sure if it would be advantageous to implement the type classes as proxies, but now realize it will be for composite data structures, i.e. `Schema`, `StructArray`, `ListArray`. ### What changes are included in this PR? 1. All classes within the `arrow.type.Type` class hierarchy are implemented as proxies. ### Are these changes tested? Yes, we had existing tests for these classes. ### Are there any user-facing changes? No. ### Future Directions 1. In a followup PR request, we plan on integrating the proxy type classes and the array classes so that they share the same underlying C++` arrow::DataType` object. We thought doing so in this change would be too much code churn. ### Notes Thank you @ kevingurney for the help! * Closes: apache#36363 Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com> Co-authored-by: sgilmore10 <74676073+sgilmore10@users.noreply.github.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>

…ay.Array` subclasses from existing proxy ids (apache#36731) ### Rationale for this change Now that the issue apache#36363 is closed via PR apache#36419, we can initialize the `Type` property of `arrow.array.Array` subclasses from existing proxy ids. Currently, we create a new proxy `Type` object whose underlying `arrow::DataType` are semantically equal to - but not the same as - the `arrow::DataType` owned by the Array proxy. It would be preferable if the `Type` and `Array` proxy classes refer to the same `arrow::DataType` object (i.e. the same object on the heap). ### What changes are included in this PR? 1. Upgraded `libmexclass` to commit [d04f88d](mathworks/libmexclass@d04f88d). In this commit, we added a static "make-like" function to `Proxy` called `create`. 2. Modified the constructors of all `Type` objects to expect a single `Proxy` object as input. This is a breaking change and means clients are no longer expected to build `Type` objects via their constructors. Instead, we introduced standalone functions that clients can use to construct `Type` objects, i.e. `arrow.type.int8`, `arrow.type.string`, `arrow.type.timestamp`, etc. These functions deal with creating the `Proxy` objects to pass to the `Type` constructors. Below is an example of the new workflow for creating `Type` objects. ```matlab >> timestampType = arrow.type.timestamp(TimeUnit="second", TimeZone="America/New_York") timestampType = TimestampType with properties: ID: Timestamp ``` NOTE: We plan on enhancing the display to show the `TimeUnit` and `TimeZone` properties. 3. Made `Type` a [dependent](https://www.mathworks.com/help/matlab/matlab_oop/access-methods-for-dependent-properties.html) property on `arrow.array.Array`. The `get.Type` method constructs a `Type` object on demand by making a proxy that wraps the same `arrow::DataType` object stored within the `arrow::Array`. ### Are these changes tested? Yes, updated existing tests. ### Are there any user-facing changes? Yes, we added new standalone functions for creating `Type` objects. Below is a table mapping standalone functions to the `Type` object they output: | Standalone Function | Output Type Object | |----------------------|---------------------| |`arrow.type.boolean`| `arrow.type.BooleanType`| |`arrow.type.int8`| `arrow.type.Int8Type`| |`arrow.type.int16`| `arrow.type.Int16Type`| |`arrow.type.int32`| `arrow.type.Int32Type`| |`arrow.type.int64`| `arrow.type.Int64Type`| |`arrow.type.uint8`| `arrow.type.UInt8Type`| |`arrow.type.uint16`| `arrow.type.UInt16Type`| |`arrow.type.uint32`| `arrow.type.UInt32Type`| |`arrow.type.uint64`| `arrow.type.UInt64Type`| |`arrow.type.string`| `arrow.type.StringType`| |`arrow.type.timestamp`| `arrow.type.TimestampType`| ### Notes Thanks @ kevingurney for the advice! * Closes: apache#36652 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>

github-actions bot added Component: MATLAB awaiting review Awaiting review labels Jun 30, 2023

sgilmore10 force-pushed the GH-36363 branch from 3215150 to 040f5ba Compare July 6, 2023 13:34

sgilmore10 added 10 commits July 7, 2023 10:24

Make Type proxies

d6bee5e

Add Float32Type and Float64Type proxy classes

620019e

Add using statements for enable_if_primitive

a1ae131

Re-implement NumericType and BooleanType classes to use proxies

1a5bf59

Update type tests

d9a5a95

Add proxy class for TimestampType

c2d6cd9

Add FixedWidthType superclass

25b10f3

Add FixedWidthType MATLAB class

7cdc10b

Add StringType proxy class

ea4df1a

Use u16string_view instead of u16string

e9cdc32

sgilmore10 force-pushed the GH-36363 branch from 040f5ba to e9cdc32 Compare July 7, 2023 14:34

sgilmore10 marked this pull request as ready for review July 7, 2023 14:34

sgilmore10 requested a review from assignUser as a code owner July 7, 2023 14:34

sgilmore10 mentioned this pull request Jul 7, 2023

[MATLAB] arrow.array.Array subclasses should instantiate the Type property from a proxy ID #36555

Closed

kevingurney self-requested a review July 7, 2023 17:06

kevingurney approved these changes Jul 7, 2023

View reviewed changes

matlab/src/cpp/arrow/matlab/type/proxy/type.h Outdated Show resolved Hide resolved

matlab/src/cpp/arrow/matlab/type/proxy/type.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 7, 2023

sgilmore10 added 2 commits July 7, 2023 13:31

Rename getType to unwrap

7cb788c

Remove unnecessary comment

75d3677

assignUser removed their request for review July 7, 2023 21:49

kou reviewed Jul 8, 2023

View reviewed changes

matlab/src/cpp/arrow/matlab/proxy/factory.cc Outdated Show resolved Hide resolved

matlab/src/cpp/arrow/matlab/type/proxy/type.cc Outdated Show resolved Hide resolved

matlab/src/cpp/arrow/matlab/type/proxy/type.cc Outdated Show resolved Hide resolved

github-actions bot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Jul 8, 2023

kou reviewed Jul 9, 2023

View reviewed changes

matlab/src/cpp/arrow/matlab/type/proxy/primitive_ctype.h Outdated Show resolved Hide resolved

matlab/src/cpp/arrow/matlab/type/proxy/primitive_ctype.h Outdated Show resolved Hide resolved

format REGISTER_PROXY command

754c5b0

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

Remove extra newline

25b8cb7

Co-authored-by: Sutou Kouhei <kou@cozmixng.org>

github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 10, 2023

Use std::move to avoid copy of shared pointer

5937040

kevingurney merged commit 6baf6a7 into apache:main Jul 12, 2023

kevingurney removed the awaiting change review Awaiting change review label Jul 12, 2023

This was referenced Jul 12, 2023

[MATLAB] Initialize the Type property of arrow.array.Array subclasses from existing proxy ids #36652

Closed

GH-36652: [MATLAB] Initialize the Type property of arrow.array.Array subclasses from existing proxy ids #36731

Merged

sgilmore10 deleted the GH-36363 branch August 21, 2023 18:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-36363: [MATLAB] Create proxy classes for the DataType class hierarchy #36419

GH-36363: [MATLAB] Create proxy classes for the DataType class hierarchy #36419

sgilmore10 commented Jun 30, 2023 •

edited

Loading

kevingurney commented Jul 7, 2023 •

edited

Loading

kevingurney left a comment

conbench-apache-arrow bot commented Jul 22, 2023

GH-36363: [MATLAB] Create proxy classes for the DataType class hierarchy #36419

GH-36363: [MATLAB] Create proxy classes for the DataType class hierarchy #36419

Conversation

sgilmore10 commented Jun 30, 2023 • edited Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Future Directions

Notes

kevingurney commented Jul 7, 2023 • edited Loading

kevingurney left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Jul 22, 2023

sgilmore10 commented Jun 30, 2023 •

edited

Loading

kevingurney commented Jul 7, 2023 •

edited

Loading