sgilmore10 opened a new pull request, #36108:
URL: https://github.com/apache/arrow/pull/36108
<!--
Thanks for opening a pull request!
If this is your first pull request you can find detailed information on how
to contribute here:
* [New Contributor's
Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
* [Contributing
Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
If this is not a [minor
PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes).
Could you open an issue for this pull request on GitHub?
https://github.com/apache/arrow/issues/new/choose
Opening GitHub issues ahead of time contributes to the
[Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.)
of the Apache Arrow project.
Then could you also rename the pull request title in the following format?
GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
or
MINOR: [${COMPONENT}] ${SUMMARY}
In the case of PARQUET issues on JIRA the title also supports:
PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
-->
### Rationale for this change
It would be better if we passed an option struct to the C++ proxy
constructors instead of a cell array containing the input arguments. If we pass
in a struct, we can access the inputs by name. For example, if the NumericArray
proxy class accepted an options struct with fields named `MatlabArray`,
`Valid`, and `DeepCopy`, we could access the values like so in its `make`
function:
```cpp
static libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments) {
::matlab::data::StructArray opts = constructor_arguments[0];
const ::matlab::data::TypedArray<CType> data_mda =
opts[0]["MatlabArray"];
const ::matlab::data::TypedArray<bool> valid_mda =
opts[0]["Valid"];
const ::matlab::data::TypedArray<bool> make_copy_mda =
opts[0]["DeepCopy"];
}
```
It's easier to reason about the code above than the code snippet below:
```cpp
static libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments) {
const ::matlab::data::TypedArray<CType> data_mda =
constructor_arguments[0];
const ::matlab::data::TypedArray<bool> valid_mda =
constructor_arguments[1];
const ::matlab::data::TypedArray<bool> make_copy_mda =
constructor_arguments[2];
}
```
Using options structs also enables us to support syntaxes at
construction-time. We can query a field on the struct to determine which fields
we should expect to be there.
### What changes are included in this PR?
1. The `NumericArray` C++ proxy classes accepts a struct with the fields
`MatlabArray`, `Valid`, and `DeepCopy` at construction time.
2. The `BooleanArray` C++ proxy class accepts a struct with the fields
`MatlabArray` and `Valid` at construction-time.
### Are these changes tested?
Existing test cases cover these changes.
### Are there any user-facing changes?
No, these changes are not user-facing.
### Future Directions
In a followup PR, we plan on adding an `ArgumentParser` class. This would
abstract out how you access fields on scalar `matlab::data::StructArray`
objects. The `NumericArray` code would then look something like this:
```cpp
static libmexclass::proxy::MakeResult make(const
libmexclass::proxy::FunctionArguments& constructor_arguments) {
ArgumentParser args{constructor_arguments[0]};
const ::matlab::data::TypedArray<CType> data_mda =
args["MatlabArray"];
const ::matlab::data::TypedArray<bool> valid_mda =
args["Valid"];
const ::matlab::data::TypedArray<bool> make_copy_mda =
args["DeepCopy"];
}
```
### Notes
Thank you to @kevingurney for all the help and the idea for the
`ArgumentParser` class.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
