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-37825: [MATLAB] Improve arrow.type.Field display #37826

Merged
merged 4 commits into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions matlab/src/cpp/arrow/matlab/type/proxy/field.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ namespace arrow::matlab::type::proxy {
Field::Field(std::shared_ptr<arrow::Field> field) : field{std::move(field)} {
REGISTER_METHOD(Field, getName);
REGISTER_METHOD(Field, getType);
REGISTER_METHOD(Field, toString);
}

std::shared_ptr<arrow::Field> Field::unwrap() {
Expand Down Expand Up @@ -64,16 +63,6 @@ namespace arrow::matlab::type::proxy {
context.outputs[0] = output;
}

void Field::toString(libmexclass::proxy::method::Context& context) {
namespace mda = ::matlab::data;
mda::ArrayFactory factory;

const auto str_utf8 = field->ToString();
MATLAB_ASSIGN_OR_ERROR_WITH_CONTEXT(const auto str_utf16, arrow::util::UTF8StringToUTF16(str_utf8), context, error::UNICODE_CONVERSION_ERROR_ID);
auto str_mda = factory.createScalar(str_utf16);
context.outputs[0] = str_mda;
}

libmexclass::proxy::MakeResult Field::make(const libmexclass::proxy::FunctionArguments& constructor_arguments) {
namespace mda = ::matlab::data;
using FieldProxy = arrow::matlab::type::proxy::Field;
Expand Down
1 change: 0 additions & 1 deletion matlab/src/cpp/arrow/matlab/type/proxy/field.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class Field : public libmexclass::proxy::Proxy {
protected:
void getName(libmexclass::proxy::method::Context& context);
void getType(libmexclass::proxy::method::Context& context);
void toString(libmexclass::proxy::method::Context& context);

std::shared_ptr<arrow::Field> field;
};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
%MAKEDIMENSIONSTRING Utility function for creating a string representation
%of dimensions.

% Licensed to the Apache Software Foundation (ASF) under one or more
% contributor license agreements. See the NOTICE file distributed with
% this work for additional information regarding copyright ownership.
% The ASF licenses this file to you under the Apache License, Version
% 2.0 (the "License"); you may not use this file except in compliance
% with the License. You may obtain a copy of the License at
%
% http://www.apache.org/licenses/LICENSE-2.0
%
% Unless required by applicable law or agreed to in writing, software
% distributed under the License is distributed on an "AS IS" BASIS,
% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
% implied. See the License for the specific language governing
% permissions and limitations under the License.

function dimensionString = makeDimensionString(arraySize)
dimensionString = string(arraySize);
dimensionString = join(dimensionString, char(215));
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
%MAKELINKSTRING Utility function for creating hyperlinks.

% Licensed to the Apache Software Foundation (ASF) under one or more
% contributor license agreements. See the NOTICE file distributed with
% this work for additional information regarding copyright ownership.
% The ASF licenses this file to you under the Apache License, Version
% 2.0 (the "License"); you may not use this file except in compliance
% with the License. You may obtain a copy of the License at
%
% http://www.apache.org/licenses/LICENSE-2.0
%
% Unless required by applicable law or agreed to in writing, software
% distributed under the License is distributed on an "AS IS" BASIS,
% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
% implied. See the License for the specific language governing
% permissions and limitations under the License.

function link = makeLinkString(opts)
arguments
opts.FullClassName(1, 1) string
opts.ClassName(1, 1) string
% When displaying heterogeneous arrays, only the name of the
% closest shared anscestor class is displayed in bold. All other
% class names are not bolded.
opts.BoldFont(1, 1) logical
end

if opts.BoldFont
link = compose("<a href=""matlab:helpPopup %s""" + ...
" style=""font-weight:bold"">%s</a>", ...
opts.FullClassName, opts.ClassName);
else
link = compose("<a href=""matlab:helpPopup %s"">%s</a>", ...
opts.FullClassName, opts.ClassName);
end
end
32 changes: 32 additions & 0 deletions matlab/src/matlab/+arrow/+internal/+test/+display/verify.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
%VERIFY Utility function used to verify object display.

% Licensed to the Apache Software Foundation (ASF) under one or more
% contributor license agreements. See the NOTICE file distributed with
% this work for additional information regarding copyright ownership.
% The ASF licenses this file to you under the Apache License, Version
% 2.0 (the "License"); you may not use this file except in compliance
% with the License. You may obtain a copy of the License at
%
% http://www.apache.org/licenses/LICENSE-2.0
%
% Unless required by applicable law or agreed to in writing, software
% distributed under the License is distributed on an "AS IS" BASIS,
% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
% implied. See the License for the specific language governing
% permissions and limitations under the License.

function verify(testCase, actualDisplay, expectedDisplay)
% When the MATLAB GUI is running, '×' (char(215)) is used as
% the delimiter between dimension values. However, when the
% GUI is not running, 'x' (char(120)) is used as the delimiter.
% To account for this discrepancy, check if actualDisplay
% contains char(215). If not, replace all instances of
% char(215) in expectedDisplay with char(120).

tf = contains(actualDisplay, char(215));
if ~tf
idx = strfind(expectedDisplay, char(215));
expectedDisplay(idx) = char(120);
end
testCase.verifyEqual(actualDisplay, expectedDisplay);
end
12 changes: 3 additions & 9 deletions matlab/src/matlab/+arrow/+type/Field.m
Original file line number Diff line number Diff line change
Expand Up @@ -91,16 +91,10 @@
end
end

methods (Access = private)
function str = toString(obj)
str = obj.Proxy.toString();
end
end

methods (Access=protected)
function displayScalarObject(obj)
disp(obj.toString());
function groups = getPropertyGroups(~)
targets = ["Name", "Type"];
groups = matlab.mixin.util.PropertyGroup(targets);
end
end

end
28 changes: 28 additions & 0 deletions matlab/test/arrow/type/tField.m
Original file line number Diff line number Diff line change
Expand Up @@ -231,5 +231,33 @@ function TestIsEqualNonScalarFalse(testCase)
% Compare arrow.type.Field array and a string array
testCase.verifyFalse(isequal(f1, strings(size(f1))));
end

function TestDisplay(testCase)
% Verify the display of Field objects.
%
% Example:
%
% Field with properties:
%
% Name: FieldA
% Type: [1x2 arrow.type.Int32Type]

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString
import arrow.internal.test.display.makeDimensionString

field = arrow.field("B", arrow.timestamp(TimeZone="America/Anchorage")); %#ok<NASGU>
classnameLink = makeLinkString(FullClassName="arrow.type.Field", ClassName="Field", BoldFont=true);
header = " " + classnameLink + " with properties:" + newline;
body = strjust(pad(["Name:"; "Type:"]));
dimensionString = makeDimensionString([1 1]);
fieldString = compose("[%s %s]", dimensionString, "arrow.type.TimestampType");
body = body + " " + ["""B"""; fieldString];
body = " " + body;
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(field)');
verify(testCase, actualDisplay, expectedDisplay);
end
end
end
85 changes: 36 additions & 49 deletions matlab/test/arrow/type/tTypeDisplay.m
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ function EmptyTypeDisplay(testCase)
%
% ID

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString
import arrow.internal.test.display.makeDimensionString

type = arrow.type.Type.empty(0, 1);
typeLink = makeLinkString(FullClassName="arrow.type.Type", ClassName="Type", BoldFont=true);
dimensionString = makeDimensionString(size(type));
Expand All @@ -59,7 +63,7 @@ function EmptyTypeDisplay(testCase)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyDisplay(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function NonScalarArrayDifferentTypes(testCase)
Expand All @@ -71,6 +75,10 @@ function NonScalarArrayDifferentTypes(testCase)
%
% ID

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString
import arrow.internal.test.display.makeDimensionString

float32Type = arrow.float32();
timestampType = arrow.timestamp();
typeArray = [float32Type timestampType];
Expand All @@ -88,7 +96,7 @@ function NonScalarArrayDifferentTypes(testCase)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(typeArray)');
testCase.verifyDisplay(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function NonScalarArraySameTypes(testCase)
Expand All @@ -102,6 +110,10 @@ function NonScalarArraySameTypes(testCase)
% TimeUnit
% TimeZone

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString
import arrow.internal.test.display.makeDimensionString

timestampType1 = arrow.timestamp(TimeZone="Pacific/Fiji");
timestampType2 = arrow.timestamp(TimeUnit="Second");
typeArray = [timestampType1 timestampType2];
Expand All @@ -114,7 +126,7 @@ function NonScalarArraySameTypes(testCase)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(typeArray)');
testCase.verifyDisplay(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function TestTypeDisplaysOnlyID(testCase, TypeDisplaysOnlyID)
Expand All @@ -127,6 +139,9 @@ function TestTypeDisplaysOnlyID(testCase, TypeDisplaysOnlyID)
%
% ID: Boolean

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString

type = TypeDisplaysOnlyID;
fullClassName = string(class(type));
className = reverse(extractBefore(reverse(fullClassName), "."));
Expand All @@ -136,7 +151,7 @@ function TestTypeDisplaysOnlyID(testCase, TypeDisplaysOnlyID)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyDisplay(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function TestTimeType(testCase, TimeType)
Expand All @@ -149,6 +164,9 @@ function TestTimeType(testCase, TimeType)
% ID: Time32
% TimeUnit: Second

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString

type = TimeType;
fullClassName = string(class(type));
className = reverse(extractBefore(reverse(fullClassName), "."));
Expand All @@ -161,7 +179,7 @@ function TestTimeType(testCase, TimeType)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyEqual(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function TestDateType(testCase, DateType)
Expand All @@ -174,6 +192,9 @@ function TestDateType(testCase, DateType)
% ID: Date32
% DateUnit: Day

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString

type = DateType;
fullClassName = string(class(type));
className = reverse(extractBefore(reverse(fullClassName), "."));
Expand All @@ -186,7 +207,7 @@ function TestDateType(testCase, DateType)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyEqual(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function TimestampTypeDisplay(testCase)
Expand All @@ -200,6 +221,9 @@ function TimestampTypeDisplay(testCase)
% TimeUnit: Second
% TimeZone: "America/Anchorage"

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString

type = arrow.timestamp(TimeUnit="Second", TimeZone="America/Anchorage"); %#ok<NASGU>
classnameLink = makeLinkString(FullClassName="arrow.type.TimestampType", ClassName="TimestampType", BoldFont=true);
header = " " + classnameLink + " with properties:" + newline;
Expand All @@ -209,7 +233,7 @@ function TimestampTypeDisplay(testCase)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyEqual(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end

function StructTypeDisplay(testCase)
Expand All @@ -222,6 +246,10 @@ function StructTypeDisplay(testCase)
% ID: Struct
% Fields: [1x2 arrow.type.Field]

import arrow.internal.test.display.verify
import arrow.internal.test.display.makeLinkString
import arrow.internal.test.display.makeDimensionString

fieldA = arrow.field("A", arrow.int32());
fieldB = arrow.field("B", arrow.timestamp(TimeZone="America/Anchorage"));
type = arrow.struct(fieldA, fieldB); %#ok<NASGU>
Expand All @@ -235,48 +263,7 @@ function StructTypeDisplay(testCase)
footer = string(newline);
expectedDisplay = char(strjoin([header body' footer], newline));
actualDisplay = evalc('disp(type)');
testCase.verifyDisplay(actualDisplay, expectedDisplay);
verify(testCase, actualDisplay, expectedDisplay);
end
end

methods
function verifyDisplay(testCase, actualDisplay, expectedDisplay)
% When the MATLAB GUI is running, '×' (char(215)) is used as
% the delimiter between dimension values. However, when the
% GUI is not running, 'x' (char(120)) is used as the delimiter.
% To account for this discrepancy, check if actualDisplay
% contains char(215). If not, replace all instances of
% char(215) in expectedDisplay with char(120).

tf = contains(actualDisplay, char(215));
if ~tf
idx = strfind(expectedDisplay, char(215));
expectedDisplay(idx) = char(120);
end
testCase.verifyEqual(actualDisplay, expectedDisplay);
end
end
end

function link = makeLinkString(opts)
arguments
opts.FullClassName(1, 1) string
opts.ClassName(1, 1) string
% When displaying heterogeneous arrays, only the name of the
% closest shared anscestor class is displayed in bold. All other
% class names are not bolded.
opts.BoldFont(1, 1) logical
end

if opts.BoldFont
link = compose("<a href=""matlab:helpPopup %s"" style=""font-weight:bold"">%s</a>", ...
opts.FullClassName, opts.ClassName);
else
link = compose("<a href=""matlab:helpPopup %s"">%s</a>", opts.FullClassName, opts.ClassName);
end
end

function dimensionString = makeDimensionString(arraySize)
dimensionString = string(arraySize);
dimensionString = join(dimensionString, char(215));
end