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

GH-37245: [MATLAB] arrow.internal.proxy.validate throws MATLAB:UndefinedFunction when crafting the message to display when throwing the arrow:proxy:ProxyNameMismatch error #37248

Merged
merged 3 commits into from
Aug 18, 2023

Conversation

sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented Aug 18, 2023

Rationale for this change

When constructing the message to display for with the arrow:proxy:ProxyNameMismatch error, we refer to an undefined variable in arrow.internal.proxy.validate(). This causes the function to throw a MATLAB:UndefinedFunction error instead of the intended one.

Here's an example of this bug:

>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Unrecognized function or variable 'proxyName'.

Error in arrow.internal.proxy.validate (line 26)
        msg = "Proxy class name is " + proxyName + ", but expected " + expectedProxyName;

This was the expected error message:

>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Error using arrow.internal.proxy.validate
Proxy class name is arrow.array.proxy.Float64Array, but expected WrongProxyName

What changes are included in this PR?

  1. Fixed the typos in arrow.internal.proxy.validate() to resolve the MATLAB:UndefinedFunction error
  2. Added a new test class test/arrow/internal/proxy/tValidate.m to test arrow.internal.proxy.validate()
  3. Updated the error message to display when throwing an arrow:proxy:ProxyNameMismatch error:
>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Error using arrow.internal.proxy.validate
The Name property of the Proxy provided is "arrow.array.proxy.Float64Array", but
expected it to be "WrongProxyName".

Are these changes tested?

Yes, added a new test class test/arrow/internal/proxy/tValidate.m.

Are there any user-facing changes?

No.

@github-actions
Copy link

⚠️ GitHub issue #37245 has been automatically assigned in GitHub to PR creator.

@kevingurney kevingurney changed the title GH-37245: [MATLAB] arrow.internal.proxy.validate throws an MATLAB:UndefinedFunction when crafting the message to display when throwing the arrow:proxy:ProxyNameMismatch error GH-37245: [MATLAB] arrow.internal.proxy.validate throws MATLAB:UndefinedFunction when crafting the message to display when throwing the arrow:proxy:ProxyNameMismatch error Aug 18, 2023
Copy link
Member

@kevingurney kevingurney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels Aug 18, 2023
@kevingurney kevingurney merged commit fc96fd2 into apache:main Aug 18, 2023
12 checks passed
@kevingurney kevingurney removed the awaiting merge Awaiting merge label Aug 18, 2023
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit fc96fd2.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@sgilmore10 sgilmore10 deleted the GH-37245 branch August 21, 2023 18:12
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…AB:UndefinedFunction` when crafting the message to display when throwing the `arrow:proxy:ProxyNameMismatch` error (apache#37248)

### Rationale for this change

When constructing the message to display for with the `arrow:proxy:ProxyNameMismatch` error,  we refer to an undefined variable in `arrow.internal.proxy.validate()`. This causes the function to throw a `MATLAB:UndefinedFunction` error instead of the intended one.

Here's an example of this bug:

```matlab
>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Unrecognized function or variable 'proxyName'.

Error in arrow.internal.proxy.validate (line 26)
        msg = "Proxy class name is " + proxyName + ", but expected " + expectedProxyName;
```

This was the expected error message:

```matlab
>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Error using arrow.internal.proxy.validate
Proxy class name is arrow.array.proxy.Float64Array, but expected WrongProxyName
```

### What changes are included in this PR?

1. Fixed the typos in `arrow.internal.proxy.validate()` to resolve the `MATLAB:UndefinedFunction` error
2. Added a new test class `test/arrow/internal/proxy/tValidate.m` to test  `arrow.internal.proxy.validate()`
3. Updated the error message to display when throwing an `arrow:proxy:ProxyNameMismatch` error: 

```matlab
>> a = arrow.array([1 2 3]);
>> arrow.internal.proxy.validate(a.Proxy, "WrongProxyName")
Error using arrow.internal.proxy.validate
The Name property of the Proxy provided is "arrow.array.proxy.Float64Array", but
expected it to be "WrongProxyName".
```

### Are these changes tested?

Yes, added a new test class `test/arrow/internal/proxy/tValidate.m`.

### Are there any user-facing changes?

No.

* Closes: apache#37245

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