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

GH-37345: [MATLAB] Add function handle to fromMATLAB static construction methods to TypeTraits classes #37370

Merged
merged 11 commits into from
Aug 25, 2023

Conversation

sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented Aug 24, 2023

Rationale for this change

Each concrete arrow.array.Array subclass has an associated fromMATLAB static construction method. It would be helpful if the TypeTraits classes had access to these corresponding fromMATLAB methods.

This would involve adding a new property like ArrayStaticConstructor to the TypeTraits classes.

For example:

>> arrow.type.traits.Int8Traits

ans = 

  Int8Traits with properties:

       ArrayConstructor: @arrow.array.Int8Array
         ArrayClassName: "arrow.array.Int8Array"
    ArrayProxyClassName: "arrow.array.proxy.Int8Array"
 ArrayStaticConstructor: @arrow.array.Int8Array.fromMATLAB
        TypeConstructor: @arrow.type.Int8Type
          TypeClassName: "arrow.type.Int8Type"
     TypeProxyClassName: "arrow.type.proxy.Int8Type"
      MatlabConstructor: @int8
        MatlabClassName: "int8"

What changes are included in this PR?

  1. Added a new abstract property called ArrayStaticConstructor to the parent class arrow.type.traits.TypeTraits.
  2. Defined the ArrayStaticConstructor property in all concrete classes of ArrayStaticConstructor.

Are these changes tested?

Yes. Added a new test case to abstract test class hTypeTraits.m.

Are there any user-facing changes?

No.

@github-actions github-actions bot added the awaiting review Awaiting review label Aug 24, 2023
Copy link
Member

@kevingurney kevingurney left a comment

Choose a reason for hiding this comment

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

Looks good!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Aug 25, 2023
@kevingurney
Copy link
Member

+1

@kevingurney kevingurney merged commit 4586689 into apache:main Aug 25, 2023
10 checks passed
@kevingurney kevingurney deleted the GH-37345 branch August 25, 2023 20:25
@kevingurney kevingurney removed the awaiting merge Awaiting merge label Aug 25, 2023
@conbench-apache-arrow
Copy link

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

There was 1 benchmark result indicating a performance regression:

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…onstruction methods to `TypeTraits` classes (apache#37370)

### Rationale for this change

Each concrete `arrow.array.Array` subclass has an associated `fromMATLAB `static construction method. It would be helpful if the `TypeTraits` classes had access to these corresponding fromMATLAB methods.

This would involve adding a new property like `ArrayStaticConstructor` to the TypeTraits classes.

For example:

```matlab
>> arrow.type.traits.Int8Traits

ans = 

  Int8Traits with properties:

       ArrayConstructor: @ arrow.array.Int8Array
         ArrayClassName: "arrow.array.Int8Array"
    ArrayProxyClassName: "arrow.array.proxy.Int8Array"
 ArrayStaticConstructor: @ arrow.array.Int8Array.fromMATLAB
        TypeConstructor: @ arrow.type.Int8Type
          TypeClassName: "arrow.type.Int8Type"
     TypeProxyClassName: "arrow.type.proxy.Int8Type"
      MatlabConstructor: @ int8
        MatlabClassName: "int8"
```
### What changes are included in this PR?

1. Added a new abstract property called `ArrayStaticConstructor` to the parent class `arrow.type.traits.TypeTraits`.
2. Defined the `ArrayStaticConstructor` property in all concrete classes of `ArrayStaticConstructor`.

### Are these changes tested?

Yes. Added a new test case to abstract test class `hTypeTraits.m`.

### Are there any user-facing changes?

No.

* Closes: apache#37345

Authored-by: Sarah Gilmore <[email protected]>
Signed-off-by: Kevin Gurney <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[MATLAB] Add function handle to fromMATLAB static construction methods to TypeTraits classes
2 participants