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-37835: [MATLAB] Improve arrow.tabular.Schema display #37836

Merged
merged 17 commits into from
Sep 29, 2023

Conversation

sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented Sep 22, 2023

Rationale for this change

We would like to change how arrow.tabular.Schemas are displayed in the Command Window. Below is the current display:

>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>>s = arrow.schema([field1 field2])

s = 

A: time32[s]
B: bool

This display is not very MATLAB-like.

What changes are included in this PR?

  1. Updated the display of arrow.tabular.Schema. Below is the new display:
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean

When MATLAB is opened in desktop mode, Schema, Time32, and Boolean are hyperlinks users can click on to view the help text for the different class types.

Are these changes tested?

Yes. Added three new test cases to tSchema.m:

  1. TestDisplaySchemaZeroFields
  2. TestDisplaySchemaOneField
  3. TestDisplaySchemaMultipleFields

Are there any user-facing changes?

Yes. arrow.tabular.Schemas will be displayed differently in the Command Window.

Notes

Once #37826 is merged, I will rebase my changes and mark this PR as ready for review.

@sgilmore10 sgilmore10 marked this pull request as ready for review September 25, 2023 15:58
@kevingurney
Copy link
Member

I haven't looked at the code yet in detail, but just a thought - what if we made the display look similar to the following?

   Arrow tabular Schema with 3 fields:

   ┌─────────────────┬─────────────────┬──────────────┐
   │ Name: AName: BName: C      │
   │ Type: Time32Type: BooleanType: String │
   └─────────────────┴─────────────────┴──────────────┘

This is just a suggestion. Let me know what you think.

@sgilmore10
Copy link
Member Author

sgilmore10 commented Sep 25, 2023

I haven't looked at the code yet in detail, but just a thought - what if we made the display look similar to the following?

   Arrow tabular Schema with 3 fields:

   ┌─────────────────┬─────────────────┬──────────────┐
   │ Name: AName: BName: C      │
   │ Type: Time32Type: BooleanType: String │
   └─────────────────┴─────────────────┴──────────────┘

This is just a suggestion. Let me know what you think.

I like this. It makes it clear this object is an Arrow Schema and not a MATLAB table. I can see how the original design may confuse users in that sense.

@kevingurney
Copy link
Member

I discussed this more with @sgilmore10, and we realized that the Unicode box-drawing characters don't align properly when certain Unicode characters are used. So, for now, we think it is best to go with a simpler display.

@sgilmore10
Copy link
Member Author

I discussed this more with @sgilmore10, and we realized that the Unicode box-drawing characters don't align properly when certain Unicode characters are used. So, for now, we think it is best to go with a simpler display.

Based on our discussion, I've updated the Schema display to the following:

>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean

When in desktop mode, Schema, Time32, and Boolean are hyperlinks to the help text for each class type.

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.

Thank you for working on this, @sgilmore10! This is a great improvement to the display! Thank you for iterating on the design with me!

@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Sep 29, 2023
@github-actions github-actions bot removed the awaiting changes Awaiting changes label Sep 29, 2023
@github-actions github-actions bot added awaiting change review Awaiting change review awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Sep 29, 2023
@github-actions github-actions bot added awaiting change review Awaiting change review awaiting merge Awaiting merge and removed awaiting changes Awaiting changes awaiting change review Awaiting change review labels Sep 29, 2023
@kevingurney
Copy link
Member

+1

@kevingurney kevingurney merged commit 00efb06 into apache:main Sep 29, 2023
9 checks passed
@kevingurney kevingurney deleted the GH-37835 branch September 29, 2023 20:16
@kevingurney kevingurney removed the awaiting merge Awaiting merge label Sep 29, 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 00efb06.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 23, 2023
…he#37836)

### Rationale for this change

We would like to change how `arrow.tabular.Schema`s are displayed in the Command Window. Below is the current display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>>s = arrow.schema([field1 field2])

s = 

A: time32[s]
B: bool
```

This display is not very MATLAB-like.

### What changes are included in this PR?

1. Updated the display of `arrow.tabular.Schema`. Below is the new display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean
```

When MATLAB is opened in desktop mode, `Schema`, `Time32`, and `Boolean` are hyperlinks users can click on to view the help text for the different class types. 

### Are these changes tested?

Yes. Added three new test cases to `tSchema.m`: 
1. `TestDisplaySchemaZeroFields`
2. `TestDisplaySchemaOneField`
3. `TestDisplaySchemaMultipleFields`

### Are there any user-facing changes?

Yes. `arrow.tabular.Schema`s will be displayed differently in the Command Window.

### Notes

Once apache#37826 is merged, I will rebase my changes and mark this PR as ready for review.
* Closes: apache#37835

Authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 23, 2023
…he#37836)

### Rationale for this change

We would like to change how `arrow.tabular.Schema`s are displayed in the Command Window. Below is the current display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>>s = arrow.schema([field1 field2])

s = 

A: time32[s]
B: bool
```

This display is not very MATLAB-like.

### What changes are included in this PR?

1. Updated the display of `arrow.tabular.Schema`. Below is the new display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean
```

When MATLAB is opened in desktop mode, `Schema`, `Time32`, and `Boolean` are hyperlinks users can click on to view the help text for the different class types. 

### Are these changes tested?

Yes. Added three new test cases to `tSchema.m`: 
1. `TestDisplaySchemaZeroFields`
2. `TestDisplaySchemaOneField`
3. `TestDisplaySchemaMultipleFields`

### Are there any user-facing changes?

Yes. `arrow.tabular.Schema`s will be displayed differently in the Command Window.

### Notes

Once apache#37826 is merged, I will rebase my changes and mark this PR as ready for review.
* Closes: apache#37835

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 pull request Nov 13, 2023
…he#37836)

### Rationale for this change

We would like to change how `arrow.tabular.Schema`s are displayed in the Command Window. Below is the current display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>>s = arrow.schema([field1 field2])

s = 

A: time32[s]
B: bool
```

This display is not very MATLAB-like.

### What changes are included in this PR?

1. Updated the display of `arrow.tabular.Schema`. Below is the new display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean
```

When MATLAB is opened in desktop mode, `Schema`, `Time32`, and `Boolean` are hyperlinks users can click on to view the help text for the different class types. 

### Are these changes tested?

Yes. Added three new test cases to `tSchema.m`: 
1. `TestDisplaySchemaZeroFields`
2. `TestDisplaySchemaOneField`
3. `TestDisplaySchemaMultipleFields`

### Are there any user-facing changes?

Yes. `arrow.tabular.Schema`s will be displayed differently in the Command Window.

### Notes

Once apache#37826 is merged, I will rebase my changes and mark this PR as ready for review.
* Closes: apache#37835

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 pull request Feb 19, 2024
…he#37836)

### Rationale for this change

We would like to change how `arrow.tabular.Schema`s are displayed in the Command Window. Below is the current display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>>s = arrow.schema([field1 field2])

s = 

A: time32[s]
B: bool
```

This display is not very MATLAB-like.

### What changes are included in this PR?

1. Updated the display of `arrow.tabular.Schema`. Below is the new display:

```matlab
>> field1 =  arrow.field("A", arrow.time32());
>> field2 = arrow.field("B", arrow.boolean());
>> s = arrow.schema([field1 field2])

s = 

  Arrow Schema with 2 fields:

    A: Time32 | B: Boolean
```

When MATLAB is opened in desktop mode, `Schema`, `Time32`, and `Boolean` are hyperlinks users can click on to view the help text for the different class types. 

### Are these changes tested?

Yes. Added three new test cases to `tSchema.m`: 
1. `TestDisplaySchemaZeroFields`
2. `TestDisplaySchemaOneField`
3. `TestDisplaySchemaMultipleFields`

### Are there any user-facing changes?

Yes. `arrow.tabular.Schema`s will be displayed differently in the Command Window.

### Notes

Once apache#37826 is merged, I will rebase my changes and mark this PR as ready for review.
* Closes: apache#37835

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
Development

Successfully merging this pull request may close these issues.

[MATLAB] Improve arrow.tabular.Schema display
2 participants