Skip to content

Commit 4041aef

Browse files
authored
chore: Consitent naming for display functions (#3871)
Signed-off-by: Robert Kruszewski <github@robertk.io>
1 parent ad8a96a commit 4041aef

File tree

17 files changed

+75
-103
lines changed

17 files changed

+75
-103
lines changed

encodings/dict/src/display.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ mod test {
2020
.into_array();
2121

2222
assert_eq!(
23-
x.display().to_string(),
23+
x.display_values().to_string(),
2424
"[\"Hello\", \"Hello\", \"Hello\", \"你好\", \"Hello\", \"Hola\"]"
2525
)
2626
}
@@ -42,7 +42,7 @@ mod test {
4242
.into_array();
4343

4444
assert_eq!(
45-
elements.display().to_string(),
45+
elements.display_values().to_string(),
4646
"[\"Hello\", \"Hello\", \"Hello\", \"你好\", \"Hello\", \"Bonjour\", \"Bonjour\", null]"
4747
);
4848

@@ -57,7 +57,7 @@ mod test {
5757
.into_array();
5858

5959
assert_eq!(
60-
lists.display().to_string(),
60+
lists.display_values().to_string(),
6161
"[[\"Hello\"], [], null, [\"Hello\", \"Hello\"], null, [\"你好\", \"Hello\"], [\"Bonjour\", \"Bonjour\", null]]"
6262
);
6363

@@ -66,7 +66,7 @@ mod test {
6666
.into_array();
6767

6868
assert_eq!(
69-
x.display().to_string(),
69+
x.display_values().to_string(),
7070
"[[\"Bonjour\", \"Bonjour\", null], [\"你好\", \"Hello\"], null, [\"Hello\", \"Hello\"], null, []]"
7171
)
7272
}

fuzz/fuzz_targets/array_ops.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ fuzz_target!(|fuzz_action: FuzzArrayAction| -> Corpus {
7272
if let Err(e) = assert_array_eq(&expected.array(), &compare_result, i) {
7373
vortex_panic!(
7474
"Failed to compare {}with {op} {v}\nError: {e}",
75-
current_array.tree_display()
75+
current_array.display_tree()
7676
)
7777
}
7878
current_array = compare_result;
@@ -82,7 +82,7 @@ fuzz_target!(|fuzz_action: FuzzArrayAction| -> Corpus {
8282
if let Err(e) = assert_array_eq(&expected.array(), &cast_result, i) {
8383
vortex_panic!(
8484
"Failed to cast {} to dtype {to}\nError: {e}",
85-
current_array.tree_display()
85+
current_array.display_tree()
8686
)
8787
}
8888
current_array = cast_result;

fuzz/fuzz_targets/file_io.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ fuzz_target!(|fuzz: FuzzFileAction| -> Corpus {
110110
{
111111
vortex_panic!(
112112
"Failed to match original array {}with{}",
113-
expected_array.tree_display(),
114-
output_array.tree_display()
113+
expected_array.display_tree(),
114+
output_array.display_tree()
115115
);
116116
}
117117
}
@@ -134,8 +134,8 @@ fn compare_struct(expected: ArrayRef, actual: ArrayRef) {
134134
comparison_result.count_set_bits(),
135135
arrow_expected.len(),
136136
"\nEXPECTED: {}ACTUAL: {}",
137-
expected.tree_display(),
138-
actual.tree_display()
137+
expected.display_tree(),
138+
actual.display_tree()
139139
);
140140
}
141141

fuzz/src/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use vortex_error::VortexError;
1010
use vortex_scalar::Scalar;
1111

1212
fn tree_display(arr: &ArrayRef) -> impl Display {
13-
arr.tree_display()
13+
arr.display_tree()
1414
}
1515

1616
#[derive(thiserror::Error)]

vortex-array/src/array/display/mod.rs

Lines changed: 45 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -12,41 +12,11 @@ use vortex_error::VortexExpect as _;
1212

1313
use crate::Array;
1414

15-
/// A shim used to display the logical contents of an array.
16-
///
17-
/// For example, an `i16` typed array containing the first five non-negative integers is displayed
18-
/// as: `[0i16, 1i16, 2i16, 3i16, 4i16]`.
19-
///
20-
/// # Examples
21-
///
22-
/// ```
23-
/// # use vortex_array::IntoArray;
24-
/// # use vortex_buffer::buffer;
25-
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
26-
/// assert_eq!(
27-
/// format!("{}", array.display()),
28-
/// "[0i16, 1i16, 2i16, 3i16, 4i16]",
29-
/// )
30-
/// ```
31-
///
32-
/// See also:
33-
/// [Array::display](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display),
34-
/// [Array::display_as](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display_as),
35-
/// [DisplayArrayAs], and [DisplayOptions].
36-
pub struct DisplayArray<'a>(pub &'a dyn Array);
37-
38-
impl Display for DisplayArray<'_> {
39-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
40-
self.0.fmt_as(f, &DisplayOptions::default())
41-
}
42-
}
43-
4415
/// Describe how to convert an array to a string.
4516
///
4617
/// See also:
47-
/// [Array::display](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display),
48-
/// [Array::display_as](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display_as),
49-
/// [DisplayArray], and [DisplayArrayAs].
18+
/// [Array::display_as](../trait.Array.html#method.display_as)
19+
/// and [DisplayArrayAs].
5020
pub enum DisplayOptions {
5121
/// Only the top-level encoding id and limited metadata: `vortex.primitive(i16, len=5)`.
5222
///
@@ -55,9 +25,8 @@ pub enum DisplayOptions {
5525
/// # use vortex_array::IntoArray;
5626
/// # use vortex_buffer::buffer;
5727
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
58-
/// let opts = DisplayOptions::MetadataOnly;
5928
/// assert_eq!(
60-
/// format!("{}", array.display_as(&opts)),
29+
/// format!("{}", array.display_as(DisplayOptions::MetadataOnly)),
6130
/// "vortex.primitive(i16, len=5)",
6231
/// );
6332
/// ```
@@ -69,53 +38,50 @@ pub enum DisplayOptions {
6938
/// # use vortex_array::IntoArray;
7039
/// # use vortex_buffer::buffer;
7140
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
72-
/// let opts = DisplayOptions::default();
7341
/// assert_eq!(
74-
/// format!("{}", array.display_as(&opts)),
42+
/// format!("{}", array.display_as(DisplayOptions::default())),
7543
/// "[0i16, 1i16, 2i16, 3i16, 4i16]",
7644
/// );
7745
/// assert_eq!(
78-
/// format!("{}", array.display_as(&opts)),
79-
/// format!("{}", array.display()),
46+
/// format!("{}", array.display_as(DisplayOptions::default())),
47+
/// format!("{}", array.display_values()),
8048
/// );
8149
/// ```
82-
CommaSeparatedScalars { space_after_comma: bool },
50+
CommaSeparatedScalars { omit_comma_after_space: bool },
8351
/// The tree of encodings and all metadata but no values.
8452
///
8553
/// ```
8654
/// # use vortex_array::display::DisplayOptions;
8755
/// # use vortex_array::IntoArray;
8856
/// # use vortex_buffer::buffer;
8957
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
90-
/// let opts = DisplayOptions::TreeDisplay;
9158
/// let expected = "root: vortex.primitive(i16, len=5) nbytes=10 B (100.00%)
9259
/// metadata: EmptyMetadata
9360
/// buffer (align=2): 10 B (100.00%)
9461
/// ";
95-
/// assert_eq!(format!("{}", array.display_as(&opts)), expected);
62+
/// assert_eq!(format!("{}", array.display_as(DisplayOptions::TreeDisplay)), expected);
9663
/// ```
9764
TreeDisplay,
9865
}
9966

10067
impl Default for DisplayOptions {
10168
fn default() -> Self {
10269
Self::CommaSeparatedScalars {
103-
space_after_comma: true,
70+
omit_comma_after_space: false,
10471
}
10572
}
10673
}
10774

10875
/// A shim used to display an array as specified in the options.
10976
///
11077
/// See also:
111-
/// [Array::display](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display),
112-
/// [Array::display_as](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display_as),
113-
/// [DisplayArray], and [DisplayOptions].
114-
pub struct DisplayArrayAs<'a>(pub &'a dyn Array, pub &'a DisplayOptions);
78+
/// [Array::display_as](../trait.Array.html#method.display_as)
79+
/// and [DisplayOptions].
80+
pub struct DisplayArrayAs<'a>(pub &'a dyn Array, pub DisplayOptions);
11581

11682
impl Display for DisplayArrayAs<'_> {
11783
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
118-
self.0.fmt_as(f, self.1)
84+
self.0.fmt_as(f, &self.1)
11985
}
12086
}
12187

@@ -138,7 +104,10 @@ impl Display for dyn Array + '_ {
138104
}
139105

140106
impl dyn Array + '_ {
141-
/// Display the logical values of the array.
107+
/// Display logical values of the array
108+
///
109+
/// For example, an `i16` typed array containing the first five non-negative integers is displayed
110+
/// as: `[0i16, 1i16, 2i16, 3i16, 4i16]`.
142111
///
143112
/// # Examples
144113
///
@@ -147,27 +116,35 @@ impl dyn Array + '_ {
147116
/// # use vortex_buffer::buffer;
148117
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
149118
/// assert_eq!(
150-
/// format!("{}", array.display()),
119+
/// format!("{}", array.display_values()),
151120
/// "[0i16, 1i16, 2i16, 3i16, 4i16]",
152121
/// )
153122
/// ```
154-
pub fn display(&self) -> DisplayArray {
155-
DisplayArray(self)
123+
///
124+
/// See also:
125+
/// [Array::display_as](..//trait.Array.html#method.display_as),
126+
/// [DisplayArrayAs], and [DisplayOptions].
127+
pub fn display_values(&self) -> DisplayArrayAs<'_> {
128+
DisplayArrayAs(
129+
self,
130+
DisplayOptions::CommaSeparatedScalars {
131+
omit_comma_after_space: false,
132+
},
133+
)
156134
}
157135

158136
/// Display the array as specified by the options.
159137
///
160138
/// See [DisplayOptions] for examples.
161-
pub fn display_as<'a>(&'a self, options: &'a DisplayOptions) -> DisplayArrayAs<'a> {
139+
pub fn display_as(&self, options: DisplayOptions) -> DisplayArrayAs<'_> {
162140
DisplayArrayAs(self, options)
163141
}
164142

165143
/// Display the tree of encodings of this array as an indented lists.
166144
///
167145
/// While some metadata (such as length, bytes and validity-rate) are included, the logical
168146
/// values of the array are not displayed. To view the logical values see
169-
/// [Array::display](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display),
170-
/// [Array::display_as](http://localhost:8000/doc/vortex_array/trait.Array.html#method.display_as),
147+
/// [Array::display_as](../trait.Array.html#method.display_as)
171148
/// and [DisplayOptions].
172149
///
173150
/// # Examples
@@ -176,25 +153,17 @@ impl dyn Array + '_ {
176153
/// # use vortex_array::IntoArray;
177154
/// # use vortex_buffer::buffer;
178155
/// let array = buffer![0_i16, 1, 2, 3, 4].into_array();
179-
/// let opts = DisplayOptions::TreeDisplay;
180156
/// let expected = "root: vortex.primitive(i16, len=5) nbytes=10 B (100.00%)
181157
/// metadata: EmptyMetadata
182158
/// buffer (align=2): 10 B (100.00%)
183159
/// ";
184-
/// assert_eq!(format!("{}", array.display_as(&opts)), expected);
160+
/// assert_eq!(format!("{}", array.display_tree()), expected);
185161
/// ```
186-
pub fn tree_display(&self) -> impl Display {
187-
TreeDisplayWrapper(self.to_array())
162+
pub fn display_tree(&self) -> impl Display {
163+
DisplayArrayAs(self, DisplayOptions::TreeDisplay)
188164
}
189165

190-
/// Format an array as specified by the options.
191-
///
192-
/// See [DisplayOptions] for examples.
193-
pub fn fmt_as(
194-
&self,
195-
f: &mut std::fmt::Formatter,
196-
options: &DisplayOptions,
197-
) -> std::fmt::Result {
166+
fn fmt_as(&self, f: &mut std::fmt::Formatter, options: &DisplayOptions) -> std::fmt::Result {
198167
match options {
199168
DisplayOptions::MetadataOnly => {
200169
write!(
@@ -205,9 +174,11 @@ impl dyn Array + '_ {
205174
self.len()
206175
)
207176
}
208-
DisplayOptions::CommaSeparatedScalars { space_after_comma } => {
177+
DisplayOptions::CommaSeparatedScalars {
178+
omit_comma_after_space,
179+
} => {
209180
write!(f, "[")?;
210-
let sep = if *space_after_comma { ", " } else { "," };
181+
let sep = if *omit_comma_after_space { "," } else { ", " };
211182
write!(
212183
f,
213184
"{}",
@@ -234,13 +205,13 @@ mod test {
234205
#[test]
235206
fn test_primitive() {
236207
let x = Buffer::<u32>::empty().into_array();
237-
assert_eq!(x.display().to_string(), "[]");
208+
assert_eq!(x.display_values().to_string(), "[]");
238209

239210
let x = buffer![1].into_array();
240-
assert_eq!(x.display().to_string(), "[1i32]");
211+
assert_eq!(x.display_values().to_string(), "[1i32]");
241212

242213
let x = buffer![1, 2, 3, 4].into_array();
243-
assert_eq!(x.display().to_string(), "[1i32, 2i32, 3i32, 4i32]");
214+
assert_eq!(x.display_values().to_string(), "[1i32, 2i32, 3i32, 4i32]");
244215
}
245216

246217
#[test]
@@ -253,7 +224,7 @@ mod test {
253224
)
254225
.unwrap()
255226
.into_array();
256-
assert_eq!(s.display().to_string(), "[{}, null, {}]");
227+
assert_eq!(s.display_values().to_string(), "[{}, null, {}]");
257228
}
258229

259230
#[test]
@@ -265,7 +236,7 @@ mod test {
265236
.unwrap()
266237
.into_array();
267238
assert_eq!(
268-
s.display().to_string(),
239+
s.display_values().to_string(),
269240
"[{x: 1i32, y: -1i32}, {x: 2i32, y: -2i32}, {x: 3i32, y: -3i32}, {x: 4i32, y: -4i32}]"
270241
);
271242
}
@@ -280,7 +251,7 @@ mod test {
280251
.unwrap()
281252
.into_array();
282253
assert_eq!(
283-
x.display().to_string(),
254+
x.display_values().to_string(),
284255
"[[], [1i32], null, [2i32], [3i32, 4i32]]"
285256
);
286257
}

vortex-array/src/array/display/tree.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::fmt::{self};
66
use humansize::{DECIMAL, format_size};
77

88
use crate::arrays::ChunkedEncoding;
9+
use crate::display::DisplayOptions;
910
use crate::{Array, ArrayRef, ArrayVisitor};
1011

1112
pub(super) struct TreeDisplayWrapper(pub(super) ArrayRef);
@@ -35,7 +36,7 @@ impl<'a, 'b: 'a> TreeFormatter<'a, 'b> {
3536
self,
3637
"{}: {} nbytes={} ({:.2}%)",
3738
name,
38-
array,
39+
array.display_as(DisplayOptions::MetadataOnly),
3940
format_size(nbytes, DECIMAL),
4041
100_f64 * nbytes as f64 / total_size as f64
4142
)?;

vortex-array/src/arrow/compute/to_arrow/canonical.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,7 @@ fn to_arrow_struct(array: StructArray, fields: &[FieldRef]) -> VortexResult<Arro
280280
vortex_bail!(
281281
"Field {} is non-nullable but has nulls {}",
282282
field,
283-
arr.tree_display()
283+
arr.display_tree()
284284
);
285285
}
286286

vortex-array/src/compute/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ impl ComputeFn {
123123
args.inputs
124124
.iter()
125125
.filter_map(|input| input.array())
126-
.format_with(",", |array, f| f(&array.tree_display()))
126+
.format_with(",", |array, f| f(&array.display_tree()))
127127
);
128128
}
129129
if output.len() != expected_len {

vortex-btrblocks/src/float.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,6 @@ mod tests {
385385

386386
let floats = values.into_array().to_primitive().unwrap();
387387
let compressed = FloatCompressor::compress(&floats, false, MAX_CASCADE, &[]).unwrap();
388-
println!("compressed: {}", compressed.tree_display())
388+
println!("compressed: {}", compressed.display_tree())
389389
}
390390
}

0 commit comments

Comments
 (0)