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

[MATLAB] Add utility functions for validating numeric and string index values #37124

Closed
sgilmore10 opened this issue Aug 11, 2023 · 1 comment · Fixed by #37150
Closed

[MATLAB] Add utility functions for validating numeric and string index values #37124

sgilmore10 opened this issue Aug 11, 2023 · 1 comment · Fixed by #37150

Comments

@sgilmore10
Copy link
Member

Describe the enhancement requested

There are multiple functions/methods within the MATLAB interface which accept either numeric or string index values - in particular, the field method of arrow.tabular.Schema and the column method of arrow.tabular.RecordBatch. Rather than writing the same validation code in each method, it would be better if we refactored that code into shared utility functions.

Component(s)

MATLAB

@sgilmore10
Copy link
Member Author

take

kevingurney pushed a commit that referenced this issue Aug 14, 2023
…tring index values (#37150)

### Rationale for this change

There are multiple functions/methods within the MATLAB interface which accept either numeric or string index values - in particular,  the `field` method of `arrow.tabular.Schema` and the `column` method of `arrow.tabular.RecordBatch`. Rather than writing the same validation code in each method, it would be better if we refactored that code into shared utility functions. 

### What changes are included in this PR?

Added the following three new validation functions:

1. `arrow.internal.validate.index.numeric`
2. `arrow.internal.validate.index.string`
3. `arrow.internal.validate.index.numericOrString`

Example Usage:

```matlab
>> index = arrow.internal.validate.index.numeric(0, "int32")
Error using arrow.internal.validate.index.numeric
Numeric indices must be positive integers.

>> index = arrow.internal.validate.index.numeric(5, "int32")

index =

  int32

   5

>>  index = arrow.internal.validate.index.string(string(missing))
Error using arrow.internal.validate.index.string
String indices must be nonmissing

>>  index = arrow.internal.validate.index.string('A')

index = 

    "A"

>> index = arrow.internal.validate.index.numericOrString(datetime, "int32")
Error using arrow.internal.validate.index.numericOrString
Indices must be positive integers or nonmissing strings.

>> index = arrow.internal.validate.index.numericOrString(5, "int32")

index =

  int32

   5

>> index = arrow.internal.validate.index.numericOrString('A', "int32")

index = 

    "A"
```

### Are these changes tested?

Yes. Added the following three new test classes:

1. `test/arrow/internal/validate/index/tNumeric.m`
2. `test/arrow/internal/validate/index/tString.m`
3. `test/arrow/internal/validate/index/tNumericOrString.m` 

### Are there any user-facing changes?

No.

### Future Directions

1. Once this PR is merged, we will change the the `field` method of `arrow.tabular.Schema` and the `column` method of `arrow.tabular.RecordBatch` to use these utilities. 
* Closes: #37124

Authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
@kevingurney kevingurney added this to the 14.0.0 milestone Aug 14, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
… and string index values (apache#37150)

### Rationale for this change

There are multiple functions/methods within the MATLAB interface which accept either numeric or string index values - in particular,  the `field` method of `arrow.tabular.Schema` and the `column` method of `arrow.tabular.RecordBatch`. Rather than writing the same validation code in each method, it would be better if we refactored that code into shared utility functions. 

### What changes are included in this PR?

Added the following three new validation functions:

1. `arrow.internal.validate.index.numeric`
2. `arrow.internal.validate.index.string`
3. `arrow.internal.validate.index.numericOrString`

Example Usage:

```matlab
>> index = arrow.internal.validate.index.numeric(0, "int32")
Error using arrow.internal.validate.index.numeric
Numeric indices must be positive integers.

>> index = arrow.internal.validate.index.numeric(5, "int32")

index =

  int32

   5

>>  index = arrow.internal.validate.index.string(string(missing))
Error using arrow.internal.validate.index.string
String indices must be nonmissing

>>  index = arrow.internal.validate.index.string('A')

index = 

    "A"

>> index = arrow.internal.validate.index.numericOrString(datetime, "int32")
Error using arrow.internal.validate.index.numericOrString
Indices must be positive integers or nonmissing strings.

>> index = arrow.internal.validate.index.numericOrString(5, "int32")

index =

  int32

   5

>> index = arrow.internal.validate.index.numericOrString('A', "int32")

index = 

    "A"
```

### Are these changes tested?

Yes. Added the following three new test classes:

1. `test/arrow/internal/validate/index/tNumeric.m`
2. `test/arrow/internal/validate/index/tString.m`
3. `test/arrow/internal/validate/index/tNumericOrString.m` 

### Are there any user-facing changes?

No.

### Future Directions

1. Once this PR is merged, we will change the the `field` method of `arrow.tabular.Schema` and the `column` method of `arrow.tabular.RecordBatch` to use these utilities. 
* Closes: apache#37124

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
Projects
None yet
2 participants