-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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-37231: [MATLAB] Add arrow.type.Time32Type
class and arrow.time32
construction function
#37250
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
Component: MATLAB
awaiting committer review
Awaiting committer review
labels
Aug 18, 2023
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
function. 2. Fix capitalization of arrow.internal.validate.temporal.timeUnit.
2. Add tests for Time32Type class, arrow.internal.validate.temporal.timeUnit function, and arrow.time32 construction function.
sgilmore10
approved these changes
Aug 18, 2023
github-actions
bot
added
awaiting changes
Awaiting changes
and removed
awaiting committer review
Awaiting committer review
labels
Aug 18, 2023
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
github-actions
bot
added
awaiting change review
Awaiting change review
awaiting changes
Awaiting changes
and removed
awaiting changes
Awaiting changes
awaiting change review
Awaiting change review
labels
Aug 18, 2023
github-actions
bot
added
awaiting change review
Awaiting change review
and removed
awaiting changes
Awaiting changes
labels
Aug 18, 2023
+1 |
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit ec1602c. There were no benchmark performance regressions. 🎉 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. |
This was referenced Aug 21, 2023
This was referenced Aug 21, 2023
kevingurney
added a commit
that referenced
this pull request
Aug 22, 2023
…`BitWidth`, and `ID` properties can not be modified to `hFixedWidth` test class (#37316) ### Rationale for this change As @ sgilmore10 pointed out in #37250 (comment), it makes sense to move the tests which verify that the `NumFields`, `BitWidth`, and `ID` properties cannot be set into the `hFixedWidthType` superclass, rather than having these tests implemented in `tTime32Type` and `tTime64Type`, since they are common to all fixed width types. ### What changes are included in this PR? 1. Moved tests which verify that the `BitWidth`, `NumFields`, and `ID` properties cannot be set out of the `tTime32Type` and `tTime64Type` classes and into the superclass `hFixedWidthType`. 2. Fixed typo in `tTime32Type` and `tTime64Type`. Changed variable name `schema` to `type`. ### Are these changes tested? Yes. 1. Moved tests which verify that the `BitWidth`, `NumFields`, and `ID` properties cannot be set out of the `tTime32Type` and `tTime64Type` classes and into the superclass `hFixedWidthType`. ### Are there any user-facing changes? No. This pull request only modifies tests. * Closes: #37253 Authored-by: Kevin Gurney <kgurney@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
kou
pushed a commit
that referenced
this pull request
Aug 23, 2023
### Rationale for this change Now that the `arrow.type.Time32Type` class has been added to the MATLAB Interface (#37250), we can add the `arrow.array.Time32Array` class. ### What changes are included in this PR? 1. Added a new class `arrow.array.Time32Array`. It has the following read-only properties: `Type` and `Length`. It has the following methods: `fromMATLAB()`, `toMATLAB()`, and `duration()`. For reference, `duration` arrays in MATLAB represent lengths of time in fixed-length units. Here's a [link](https://www.mathworks.com/help/matlab/ref/duration.html) to the documentation. 2. Added a new class called `arrow.type.traits.Time32Traits`. **Example Usage** ```matlab >> times = seconds([100 120 NaN 200]) times = 1×4 duration array 100 sec 120 sec NaN sec 200 sec >> time32Array = arrow.array.Time32Array.fromMATLAB(times) time32Array = [ 00:01:40, 00:02:00, null, 00:03:20 ] >> roundtrip = toMATLAB(time32Array) roundtrip = 4×1 duration array 100 sec 120 sec NaN sec 200 sec ``` You can also specify the `TimeUnit` to use via a name-value pair. `TimeUnit` can either be `"Second"` (default) or `"Milisecond"`. ```matlab >> times = seconds([100 120 NaN 200]); >> time32Array = arrow.array.Time32Array.fromMATLAB(times, TimeUnit="Millisecond") time32Array = [ 00:01:40.000, 00:02:00.000, null, 00:03:20.000 ] ``` ### Are these changes tested? Yes. Added two new test classes: `tTime32Array.m` and `tTime32Traits.m`. ### Are there any user-facing changes? Yes, users can now create `arrow.array.Time32Array`s from MATLAB `duration`s via the static `fromMATLAB` method on `arrow.array.Time32Array`. ### Future Directions 1. Add `arrow.array.Time64Array` 2. Add `arrow.type.Date32Type` and `arrow.array.Date32Array` 4. Add `arrow.type.Date64Type` and `arrow.array.Date64Array` * Closes: #37290 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
….time32` construction function (apache#37250) ### Rationale for this change To support the addition of `arrow.array.Time32Array`, this pull request adds a new `arrow.type.Time32Type` class and associated `arrow.time32` construction function to the MATLAB interface. ### What changes are included in this PR? 1. New `arrow.type.Time32Type` class. 2. New `arrow.time32` construction function that returns an `arrow.type.Time32Type` instance. **Example** ```matlab >> type = arrow.time32(TimeUnit="Millisecond") type = Time32Type with properties: ID: Time32 TimeUnit: Millisecond >> class(type) ans = 'arrow.type.Time32Type' ``` ### Are these changes tested? Yes. 1. Added new tests for `Time32` type ID enumeration to `tID`. 2. Added a test class `tTimeUnit` for the new validation function `arrow.internal.validate.temporal.timeUnit`. 4. Added a new test class `tTime32Type`. ### Are there any user-facing changes? Yes. There are two new user-facing APIs: 1. `arrow.time32` construction function 2. `arrow.type.Time32Type` class ### Future Directions: 1. apache#37232 2. apache#37229 3. apache#37230 4. apache#37251 ### Notes 1. Thank you @ sgilmore10 for your help with this pull request! * Closes: apache#37231 Lead-authored-by: Kevin Gurney <kgurney@mathworks.com> Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…lds`, `BitWidth`, and `ID` properties can not be modified to `hFixedWidth` test class (apache#37316) ### Rationale for this change As @ sgilmore10 pointed out in apache#37250 (comment), it makes sense to move the tests which verify that the `NumFields`, `BitWidth`, and `ID` properties cannot be set into the `hFixedWidthType` superclass, rather than having these tests implemented in `tTime32Type` and `tTime64Type`, since they are common to all fixed width types. ### What changes are included in this PR? 1. Moved tests which verify that the `BitWidth`, `NumFields`, and `ID` properties cannot be set out of the `tTime32Type` and `tTime64Type` classes and into the superclass `hFixedWidthType`. 2. Fixed typo in `tTime32Type` and `tTime64Type`. Changed variable name `schema` to `type`. ### Are these changes tested? Yes. 1. Moved tests which verify that the `BitWidth`, `NumFields`, and `ID` properties cannot be set out of the `tTime32Type` and `tTime64Type` classes and into the superclass `hFixedWidthType`. ### Are there any user-facing changes? No. This pull request only modifies tests. * Closes: apache#37253 Authored-by: Kevin Gurney <kgurney@mathworks.com> Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne
pushed a commit
to loicalleyne/arrow
that referenced
this pull request
Nov 13, 2023
…37315) ### Rationale for this change Now that the `arrow.type.Time32Type` class has been added to the MATLAB Interface (apache#37250), we can add the `arrow.array.Time32Array` class. ### What changes are included in this PR? 1. Added a new class `arrow.array.Time32Array`. It has the following read-only properties: `Type` and `Length`. It has the following methods: `fromMATLAB()`, `toMATLAB()`, and `duration()`. For reference, `duration` arrays in MATLAB represent lengths of time in fixed-length units. Here's a [link](https://www.mathworks.com/help/matlab/ref/duration.html) to the documentation. 2. Added a new class called `arrow.type.traits.Time32Traits`. **Example Usage** ```matlab >> times = seconds([100 120 NaN 200]) times = 1×4 duration array 100 sec 120 sec NaN sec 200 sec >> time32Array = arrow.array.Time32Array.fromMATLAB(times) time32Array = [ 00:01:40, 00:02:00, null, 00:03:20 ] >> roundtrip = toMATLAB(time32Array) roundtrip = 4×1 duration array 100 sec 120 sec NaN sec 200 sec ``` You can also specify the `TimeUnit` to use via a name-value pair. `TimeUnit` can either be `"Second"` (default) or `"Milisecond"`. ```matlab >> times = seconds([100 120 NaN 200]); >> time32Array = arrow.array.Time32Array.fromMATLAB(times, TimeUnit="Millisecond") time32Array = [ 00:01:40.000, 00:02:00.000, null, 00:03:20.000 ] ``` ### Are these changes tested? Yes. Added two new test classes: `tTime32Array.m` and `tTime32Traits.m`. ### Are there any user-facing changes? Yes, users can now create `arrow.array.Time32Array`s from MATLAB `duration`s via the static `fromMATLAB` method on `arrow.array.Time32Array`. ### Future Directions 1. Add `arrow.array.Time64Array` 2. Add `arrow.type.Date32Type` and `arrow.array.Date32Array` 4. Add `arrow.type.Date64Type` and `arrow.array.Date64Array` * Closes: apache#37290 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rationale for this change
To support the addition of
arrow.array.Time32Array
, this pull request adds a newarrow.type.Time32Type
class and associatedarrow.time32
construction function to the MATLAB interface.What changes are included in this PR?
arrow.type.Time32Type
class.arrow.time32
construction function that returns anarrow.type.Time32Type
instance.Example
Are these changes tested?
Yes.
Time32
type ID enumeration totID
.tTimeUnit
for the new validation functionarrow.internal.validate.temporal.timeUnit
.tTime32Type
.Are there any user-facing changes?
Yes.
There are two new user-facing APIs:
arrow.time32
construction functionarrow.type.Time32Type
classFuture Directions:
arrow.type.Time64Type
class andarrow.time64
construction function #37232arrow.type.Date32Type
class andarrow.date32
construction function #37229arrow.type.Date64Type
class andarrow.date64
construction function #37230arrow.type.TemporalType
a "tag" class #37251Notes
arrow.type.Time32Type
class andarrow.time32
construction function #37231