-
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
[MATLAB] Add an abstract class called arrow.type.TimeType
#37262
Comments
take |
kevingurney
pushed a commit
that referenced
this issue
Aug 21, 2023
…#37279) ### Rationale for this change To reduce code duplication within `Time32Type` and `Time64Type`, we should add an abstract class called `arrow.type.TimeType` that implements the getter method for the `TimeUnit` property. This class hierarchy will mirror the class hierarchy in the C++ arrow implementation. ### What changes are included in this PR? 1. Defined a new C++ proxy class for called `arrow::matlab::type::proxy::TimeType`. This class defines a `getTimeUnit` method. 2. Modified the C++ proxy class `Time32Type` to inherit from `arrow::matlab::type::proxy::TimeType`. Removed the `getTimeUnit` method from this class because it's now defined on `TimeType`. 3. Added a new MATLAB class called `arrow.type.TimeType`. It has one method: `get.TimeUnit`. 4. Modified `arrow.type.Time32Type` to inherit from `arrow.type.TimeType` and removed its `get.TimeUnit` method. ### Are these changes tested? Yes, the existing tests cover these changes. ### Are there any user-facing changes? No. ### Future Directions 1. #37232 2. #37229 3. #37230 * Closes: #37262 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 issue
Nov 13, 2023
…eType` (apache#37279) ### Rationale for this change To reduce code duplication within `Time32Type` and `Time64Type`, we should add an abstract class called `arrow.type.TimeType` that implements the getter method for the `TimeUnit` property. This class hierarchy will mirror the class hierarchy in the C++ arrow implementation. ### What changes are included in this PR? 1. Defined a new C++ proxy class for called `arrow::matlab::type::proxy::TimeType`. This class defines a `getTimeUnit` method. 2. Modified the C++ proxy class `Time32Type` to inherit from `arrow::matlab::type::proxy::TimeType`. Removed the `getTimeUnit` method from this class because it's now defined on `TimeType`. 3. Added a new MATLAB class called `arrow.type.TimeType`. It has one method: `get.TimeUnit`. 4. Modified `arrow.type.Time32Type` to inherit from `arrow.type.TimeType` and removed its `get.TimeUnit` method. ### Are these changes tested? Yes, the existing tests cover these changes. ### Are there any user-facing changes? No. ### Future Directions 1. apache#37232 2. apache#37229 3. apache#37230 * Closes: apache#37262 Authored-by: Sarah Gilmore <sgilmore@mathworks.com> 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
Describe the enhancement requested
To reduce code duplication within
Time32Type
andTime64Type
, we should add an abstract class calledarrow.type.TimeType
that implements the getter method for theTimeUnit
property. This class hierarchy will mirror the class hierarchy in the C++ arrow implementation.Component(s)
MATLAB
The text was updated successfully, but these errors were encountered: