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.

Sign up for GitHub

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

Jump to bottom

[MATLAB] Add utility to create proxies from existing arrow::DataType objects #36853

Closed
sgilmore10 opened this issue Jul 24, 2023 · 1 comment · Fixed by #36873
Closed

[MATLAB] Add utility to create proxies from existing arrow::DataType objects #36853

sgilmore10 opened this issue Jul 24, 2023 · 1 comment · Fixed by #36873
Assignees
@sgilmore10
Labels
Component: MATLAB Type: enhancement
Milestone
14.0.0

Comments

@sgilmore10
Copy link
Member

Describe the enhancement requested

It would be helpful to have a utility called wrap that accepts an existing arrow::DataType object and returns a Type proxy object. This utility will be helpful when we implement methods that extract nested arrow data structures from a parent object. Writing this utility will reduce code duplication.

Component(s)

MATLAB
@sgilmore10 sgilmore10 changed the title [MATLAB] Add utility to create proxies from existing arrow::DataType objects [MATLAB] Add utility to create proxies from existing arrow::DataType objects Jul 24, 2023
@sgilmore10
Copy link
Member Author

take

@kevingurney kevingurney mentioned this issue Jul 24, 2023
Merged
kevingurney added a commit that referenced this issue Jul 25, 2023
@kevingurney @sgilmore10 @kou
GH-36852: [MATLAB] Add arrow.type.Field class (#36855)
7b1e834 
### Rationale for this change

Now that the MATLAB interface supports creating `Type` objects - like `arrow.type.Int64Type`, we can add support for `Field` objects (i.e. name + type).

This mirrors the `Field` type in other Arrow interfaces, [like PyArrow](https://arrow.apache.org/docs/python/generated/pyarrow.field.html).

### What changes are included in this PR?

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function).

**Example**:

```matlab
>> field = arrow.field("Speed", arrow.type.uint64)   

field = 

Speed: uint64

>> field.Type

ans = 

  UInt64Type with properties:

    ID: UInt64

>> field.Name

ans = 

    "Speed"
```

### Are these changes tested?

Yes.

1. Added new `tField.m` MATLAB test class.

### Are there any user-facing changes?

Yes.

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function)

### Future Directions

1. We intentionally placed `arrow.field` in the top-level `arrow` package, rather than under a nested `arrow.type` package (where the corresponding class `arrow.type.Field` is). This is to avoid naming conflicts between `arrow.type.field` and `arrow.type.Field` and also to make it easier to use the recommended public/user-facing APIs with less typing (i.e. without needing to type nested package names). While working on this change, we realized that it would make sense to move the "type constructor functions" (e.g. `arrow.type.boolean`, `arrow.type.uint64`, etc.) into the top-level `arrow` package, as well (i.e. `arrow.boolean`, `arrow.uint64`, etc.). In other words, moving forward, the recommended APIs for the MATLAB interface will be placed directly under the top-level `arrow` package. This should hopefully result in a simplified public interface, as well as make it easier to use multiple language bindings (e.g. MATLAB and PyArrow) together, with less context switching. 

### Notes

1. @ sgilmore10 is working on a follow up PR (to address #36853) for simplifying the `switch` statement code in `makeTypeProxy`. Her solution will be more generic, so that we can re-use it elsewhere across the code base of the MATLAB interface.
2. Thank you @ sgilmore10 for your help with this pull request!
* Closes: #36852

Lead-authored-by: Kevin Gurney <kgurney@mathworks.com>
Co-authored-by: Kevin Gurney <kevin.p.gurney@gmail.com>
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
@github-actions github-actions bot mentioned this issue Jul 25, 2023
Merged
kevingurney pushed a commit that referenced this issue Jul 26, 2023
@sgilmore10 @kou
GH-36853: [MATLAB] Add utility to create proxies from existing `arrow…
9fe23da 
…::DataType` objects (#36873)

### Rationale for this change

There will be many places in the MATLAB interface code base in which we will have to wrap an `arrow::DataType` object within a subclass of `arrow::matlab::type::proxy::Type`. To avoid code duplicaiton, we should add a  utility function called `wrap` that accepts a pointer to an `arrow::DataType` object and returns a pointer to a  `arrow::matlab::type::proxy::Type` object. 

### What changes are included in this PR?

1. Added a new function with the following signature:

```cpp
arrow::Result<std::shared_ptr<arrow::matlab::type::proxy::Type>> wrap(const std::shared_ptr<arrow::DataType>& datatype);
```

2.  Updated the `type` methods of `arrow::matlab::type::proxy::Field` and `arrow::matlab::array::proxy::Array` to use `wrap`.

### Are these changes tested?

No new tests needed.

### Are there any user-facing changes?

No

* Closes: #36853

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>
@kevingurney kevingurney added this to the 14.0.0 milestone Jul 26, 2023
This was referenced Aug 3, 2023
Closed
Merged
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
@kevingurney @sgilmore10
@kou @R-JunmingChen
apacheGH-36852: [MATLAB] Add arrow.type.Field class (apache#36855)
a926957 
### Rationale for this change

Now that the MATLAB interface supports creating `Type` objects - like `arrow.type.Int64Type`, we can add support for `Field` objects (i.e. name + type).

This mirrors the `Field` type in other Arrow interfaces, [like PyArrow](https://arrow.apache.org/docs/python/generated/pyarrow.field.html).

### What changes are included in this PR?

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function).

**Example**:

```matlab
>> field = arrow.field("Speed", arrow.type.uint64)   

field = 

Speed: uint64

>> field.Type

ans = 

  UInt64Type with properties:

    ID: UInt64

>> field.Name

ans = 

    "Speed"
```

### Are these changes tested?

Yes.

1. Added new `tField.m` MATLAB test class.

### Are there any user-facing changes?

Yes.

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function)

### Future Directions

1. We intentionally placed `arrow.field` in the top-level `arrow` package, rather than under a nested `arrow.type` package (where the corresponding class `arrow.type.Field` is). This is to avoid naming conflicts between `arrow.type.field` and `arrow.type.Field` and also to make it easier to use the recommended public/user-facing APIs with less typing (i.e. without needing to type nested package names). While working on this change, we realized that it would make sense to move the "type constructor functions" (e.g. `arrow.type.boolean`, `arrow.type.uint64`, etc.) into the top-level `arrow` package, as well (i.e. `arrow.boolean`, `arrow.uint64`, etc.). In other words, moving forward, the recommended APIs for the MATLAB interface will be placed directly under the top-level `arrow` package. This should hopefully result in a simplified public interface, as well as make it easier to use multiple language bindings (e.g. MATLAB and PyArrow) together, with less context switching. 

### Notes

1. @ sgilmore10 is working on a follow up PR (to address apache#36853) for simplifying the `switch` statement code in `makeTypeProxy`. Her solution will be more generic, so that we can re-use it elsewhere across the code base of the MATLAB interface.
2. Thank you @ sgilmore10 for your help with this pull request!
* Closes: apache#36852

Lead-authored-by: Kevin Gurney <kgurney@mathworks.com>
Co-authored-by: Kevin Gurney <kevin.p.gurney@gmail.com>
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this issue Aug 20, 2023
@sgilmore10 @kou @R-JunmingChen
apacheGH-36853: [MATLAB] Add utility to create proxies from existing …
a10d05b 
…`arrow::DataType` objects (apache#36873)

### Rationale for this change

There will be many places in the MATLAB interface code base in which we will have to wrap an `arrow::DataType` object within a subclass of `arrow::matlab::type::proxy::Type`. To avoid code duplicaiton, we should add a  utility function called `wrap` that accepts a pointer to an `arrow::DataType` object and returns a pointer to a  `arrow::matlab::type::proxy::Type` object. 

### What changes are included in this PR?

1. Added a new function with the following signature:

```cpp
arrow::Result<std::shared_ptr<arrow::matlab::type::proxy::Type>> wrap(const std::shared_ptr<arrow::DataType>& datatype);
```

2.  Updated the `type` methods of `arrow::matlab::type::proxy::Field` and `arrow::matlab::array::proxy::Array` to use `wrap`.

### Are these changes tested?

No new tests needed.

### Are there any user-facing changes?

No

* Closes: apache#36853

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>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@kevingurney @sgilmore10
@kou @loicalleyne
apacheGH-36852: [MATLAB] Add arrow.type.Field class (apache#36855)
5d63f74 
### Rationale for this change

Now that the MATLAB interface supports creating `Type` objects - like `arrow.type.Int64Type`, we can add support for `Field` objects (i.e. name + type).

This mirrors the `Field` type in other Arrow interfaces, [like PyArrow](https://arrow.apache.org/docs/python/generated/pyarrow.field.html).

### What changes are included in this PR?

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function).

**Example**:

```matlab
>> field = arrow.field("Speed", arrow.type.uint64)   

field = 

Speed: uint64

>> field.Type

ans = 

  UInt64Type with properties:

    ID: UInt64

>> field.Name

ans = 

    "Speed"
```

### Are these changes tested?

Yes.

1. Added new `tField.m` MATLAB test class.

### Are there any user-facing changes?

Yes.

Two new user-facing APIs have been added:

1. `arrow.field(name, type)` constructor function
2. `arrow.type.Field` class (returned by the `arrow.field` constructor function)

### Future Directions

1. We intentionally placed `arrow.field` in the top-level `arrow` package, rather than under a nested `arrow.type` package (where the corresponding class `arrow.type.Field` is). This is to avoid naming conflicts between `arrow.type.field` and `arrow.type.Field` and also to make it easier to use the recommended public/user-facing APIs with less typing (i.e. without needing to type nested package names). While working on this change, we realized that it would make sense to move the "type constructor functions" (e.g. `arrow.type.boolean`, `arrow.type.uint64`, etc.) into the top-level `arrow` package, as well (i.e. `arrow.boolean`, `arrow.uint64`, etc.). In other words, moving forward, the recommended APIs for the MATLAB interface will be placed directly under the top-level `arrow` package. This should hopefully result in a simplified public interface, as well as make it easier to use multiple language bindings (e.g. MATLAB and PyArrow) together, with less context switching. 

### Notes

1. @ sgilmore10 is working on a follow up PR (to address apache#36853) for simplifying the `switch` statement code in `makeTypeProxy`. Her solution will be more generic, so that we can re-use it elsewhere across the code base of the MATLAB interface.
2. Thank you @ sgilmore10 for your help with this pull request!
* Closes: apache#36852

Lead-authored-by: Kevin Gurney <kgurney@mathworks.com>
Co-authored-by: Kevin Gurney <kevin.p.gurney@gmail.com>
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
@sgilmore10 @kou @loicalleyne
apacheGH-36853: [MATLAB] Add utility to create proxies from existing …
956ccc9 
…`arrow::DataType` objects (apache#36873)

### Rationale for this change

There will be many places in the MATLAB interface code base in which we will have to wrap an `arrow::DataType` object within a subclass of `arrow::matlab::type::proxy::Type`. To avoid code duplicaiton, we should add a  utility function called `wrap` that accepts a pointer to an `arrow::DataType` object and returns a pointer to a  `arrow::matlab::type::proxy::Type` object. 

### What changes are included in this PR?

1. Added a new function with the following signature:

```cpp
arrow::Result<std::shared_ptr<arrow::matlab::type::proxy::Type>> wrap(const std::shared_ptr<arrow::DataType>& datatype);
```

2.  Updated the `type` methods of `arrow::matlab::type::proxy::Field` and `arrow::matlab::array::proxy::Array` to use `wrap`.

### Are these changes tested?

No new tests needed.

### Are there any user-facing changes?

No

* Closes: apache#36853

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sgilmore10 sgilmore10

Labels
Component: MATLAB Type: enhancement
Projects
None yet
Milestone
14.0.0
2 participants
@kevingurney @sgilmore10