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

GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array #35491

Merged
merged 4 commits into from
May 11, 2023
Merged

GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array #35491

merged 4 commits into from
May 11, 2023

Conversation

sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented May 8, 2023 •
edited

Rationale for this change

It will be helpful for sharing the implementation of common functionality if there is an abstract base class from which the MATLAB arrow.array.[Type]Array classes can inherit.

What changes are included in this PR?

  1. Added abstract base class arrow.array.Array
  2. Changed arrow.array.Float64Array to inherit from arrow.array.Array
  3. Added Length property on arrow.array.Array

Are these changes tested?

  1. Added a test point for the Length property in tFloat64Array.m.
  2. Qualified locally on macOS and linux.

Are there any user-facing changes?

Yes, there is now a Length property on arrow.array.Float64Array

Future Directions

We will continue to build out the concrete Array subclasses.

@sgilmore10 sgilmore10 changed the title GH-35480: [MATLAB] [3:05 PM] Add abstract MATLAB base class called arrow.array.Array GH-35480: [MATLAB] Add abstract MATLAB base class called arrow.array.Array May 8, 2023
@github-actions
Copy link

github-actions bot commented May 8, 2023

@github-actions github-actions bot added the awaiting review Awaiting review label May 8, 2023
@sgilmore10 sgilmore10 mentioned this pull request May 8, 2023
Merged
sgilmore10 and others added 3 commits May 10, 2023 09:23
@sgilmore10 @kevingurney
Add abstract base class arrow.array.Array
b3d1ea6 
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
@sgilmore10 @kevingurney
1. Add abstract MATLAB base class arrow.array.Array
c32d901 
2. Implement Length()
3. Make arrow.array.Float64Array inherit from arrow.array.Arrat

Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
@kevingurney @sgilmore10
Add test point for arrow.array.Array.Length property.
8645e1b 
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
@sgilmore10 sgilmore10 marked this pull request as ready for review May 10, 2023 14:02
@sgilmore10
Copy link
Member Author

Marking PR as read for review.

@kevingurney kevingurney mentioned this pull request May 11, 2023
Merged
kou
kou approved these changes May 11, 2023
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels May 11, 2023
@kou kou merged commit cdefbb8 into apache:main May 11, 2023
10 checks passed
@ursabot
Copy link

ursabot commented May 13, 2023

Benchmark runs are scheduled for baseline = 14f9bf9 and contender = cdefbb8. cdefbb8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️2.27% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.25% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.42% ⬆️0.03%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] cdefbb8f ec2-t3-xlarge-us-east-2
[Finished] cdefbb8f test-mac-arm
[Finished] cdefbb8f ursa-i9-9960x
[Finished] cdefbb8f ursa-thinkcentre-m75q
[Finished] 14f9bf92 ec2-t3-xlarge-us-east-2
[Finished] 14f9bf92 test-mac-arm
[Finished] 14f9bf92 ursa-i9-9960x
[Finished] 14f9bf92 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
@sgilmore10 @kevingurney @ArgusLi
apacheGH-35480: [MATLAB] Add abstract MATLAB base class called `arrow…
36398da 
….array.Array` (apache#35491)

### Rationale for this change

It will be helpful for sharing the implementation of common functionality if there is an abstract base class from which the MATLAB `arrow.array.[Type]Array` classes can inherit. 

### What changes are included in this PR?

1. Added abstract base class `arrow.array.Array`
2. Changed `arrow.array.Float64Array` to inherit from `arrow.array.Array`
3. Added `Length` property on `arrow.array.Array`

### Are these changes tested?

1. Added a test point for the `Length` property in `tFloat64Array.m`.
2. Qualified locally on macOS and linux.

### Are there any user-facing changes?

Yes, there is now a `Length` property on `arrow.array.Float64Array`

### Future Directions

We will continue to build out the concrete Array subclasses. 
* Closes: apache#35480

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
rtpsw pushed a commit to rtpsw/arrow that referenced this pull request May 16, 2023
@sgilmore10 @kevingurney @rtpsw
apacheGH-35480: [MATLAB] Add abstract MATLAB base class called `arrow…
a008758 
….array.Array` (apache#35491)

### Rationale for this change

It will be helpful for sharing the implementation of common functionality if there is an abstract base class from which the MATLAB `arrow.array.[Type]Array` classes can inherit. 

### What changes are included in this PR?

1. Added abstract base class `arrow.array.Array`
2. Changed `arrow.array.Float64Array` to inherit from `arrow.array.Array`
3. Added `Length` property on `arrow.array.Array`

### Are these changes tested?

1. Added a test point for the `Length` property in `tFloat64Array.m`.
2. Qualified locally on macOS and linux.

### Are there any user-facing changes?

Yes, there is now a `Length` property on `arrow.array.Float64Array`

### Future Directions

We will continue to build out the concrete Array subclasses. 
* Closes: apache#35480

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kevingurney kevingurney mentioned this pull request Aug 16, 2023
Closed
4 tasks
@kevingurney kevingurney deleted the GH-35480 branch August 21, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kou kou kou approved these changes

@assignUser assignUser Awaiting requested review from assignUser

Assignees
No one assigned
Labels
awaiting merge Awaiting merge Component: MATLAB
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MATLAB] Add abstract MATLAB base class called arrow.array.Array
4 participants
@sgilmore10 @ursabot @kou @kevingurney