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 AllowNonScalar name-value pair to arrow.internal.validate.index.* validation functions #37477

Closed
kevingurney opened this issue Aug 30, 2023 · 1 comment · Fixed by #37482

Comments

@kevingurney
Copy link
Member

Describe the enhancement requested

Per #37475 (comment), we should consider adding a name-value pair like AllowNonScalar = true | false to the arrow.internal.validate.index.* validation functions since it is relatively common to want to explicitly allow (or disallow) non-scalar inputs to indexing functions (e.g. the column method of RecordBatch should only support scalar index values).

Component(s)

MATLAB

@sgilmore10
Copy link
Member

take

kevingurney pushed a commit that referenced this issue Aug 31, 2023
…rnal.validate.index.* validation functions (#37482)

### Rationale for this change

Per #37475 (comment), we should consider adding a name-value pair like `AllowNonScalar = true | false` to the `arrow.internal.validate.index.*` validation functions since it is relatively common to want to explicitly allow (or disallow) non-scalar inputs to indexing functions (e.g. the `column` method of `RecordBatch` should only support scalar index values).

### What changes are included in this PR?

1. Modified all functions within the `arrow.internal.valdiate.index` package (i.e. `numeric()`, `string()`, and `numericOrString()`)  to accept a name-value pair called `AllowNonScalar`. This name-value pair can be set to `logical` scalar, and by default it's set to `true`.
2. Updated the `column()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.
3. Updated the `field()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.

**NOTE:** While character row vectors (e.g. `'ABC'`) are not scalar, they are equivalent to scalar `string` arrays. Therefore, both `string()` and `numericOrString()` do not error if given a character row vector as the index to validate and `AllowNonScalar=false`. 

### Are these changes tested?

Yes. Added new test cases to `tNumeric.m`, `tString.m` and `tNumericOrString.m`

### Are there any user-facing changes?

No.

* Closes: #37477

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 31, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…w.internal.validate.index.* validation functions (apache#37482)

### Rationale for this change

Per apache#37475 (comment), we should consider adding a name-value pair like `AllowNonScalar = true | false` to the `arrow.internal.validate.index.*` validation functions since it is relatively common to want to explicitly allow (or disallow) non-scalar inputs to indexing functions (e.g. the `column` method of `RecordBatch` should only support scalar index values).

### What changes are included in this PR?

1. Modified all functions within the `arrow.internal.valdiate.index` package (i.e. `numeric()`, `string()`, and `numericOrString()`)  to accept a name-value pair called `AllowNonScalar`. This name-value pair can be set to `logical` scalar, and by default it's set to `true`.
2. Updated the `column()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.
3. Updated the `field()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.

**NOTE:** While character row vectors (e.g. `'ABC'`) are not scalar, they are equivalent to scalar `string` arrays. Therefore, both `string()` and `numericOrString()` do not error if given a character row vector as the index to validate and `AllowNonScalar=false`. 

### Are these changes tested?

Yes. Added new test cases to `tNumeric.m`, `tString.m` and `tNumericOrString.m`

### Are there any user-facing changes?

No.

* Closes: apache#37477

Authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
dgreiss pushed a commit to dgreiss/arrow that referenced this issue Feb 19, 2024
…w.internal.validate.index.* validation functions (apache#37482)

### Rationale for this change

Per apache#37475 (comment), we should consider adding a name-value pair like `AllowNonScalar = true | false` to the `arrow.internal.validate.index.*` validation functions since it is relatively common to want to explicitly allow (or disallow) non-scalar inputs to indexing functions (e.g. the `column` method of `RecordBatch` should only support scalar index values).

### What changes are included in this PR?

1. Modified all functions within the `arrow.internal.valdiate.index` package (i.e. `numeric()`, `string()`, and `numericOrString()`)  to accept a name-value pair called `AllowNonScalar`. This name-value pair can be set to `logical` scalar, and by default it's set to `true`.
2. Updated the `column()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.
3. Updated the `field()` method in `RecordBatch` to pass `AllowNonScalar=false` to `numericOrString()`.

**NOTE:** While character row vectors (e.g. `'ABC'`) are not scalar, they are equivalent to scalar `string` arrays. Therefore, both `string()` and `numericOrString()` do not error if given a character row vector as the index to validate and `AllowNonScalar=false`. 

### Are these changes tested?

Yes. Added new test cases to `tNumeric.m`, `tString.m` and `tNumericOrString.m`

### Are there any user-facing changes?

No.

* Closes: apache#37477

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