Skip to content

Commit

Permalink
Fix formatting of empty zip lists
Browse files Browse the repository at this point in the history
Empty zip lists (meaning zip lists with entities but without any
values) gave an IndexError when attempting formatting. This fixes the
problem, plus refactors the formatting code. In particular, the function
to find the elision has been refactored to work only on the 'values'
portion of the table to reduce the amount of 'magic numbers', previously
needed to account for the table keys

Resolves #339
  • Loading branch information
pvandyken committed Dec 12, 2023
1 parent 332ee47 commit b8f7898
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 25 deletions.
44 changes: 28 additions & 16 deletions snakebids/io/printing.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import operator as op
import textwrap
from math import ceil, floor, inf
from typing import Sequence

import more_itertools as itx

Expand All @@ -23,12 +24,24 @@ def quote_wrap(val: str) -> str:
def format_zip_lists(
zip_list: ZipList, max_width: int | float | None = None, tabstop: int = 4
) -> str:
if not zip_list:
return "{}"
table = [_format_zip_row(key, row) for key, row in zip_list.items()]
widths = [max(len(val) for val in col) for col in zip(*table)]
aligned = _align_zip_table(table, widths)
cols: list[Sequence[str]] = list(zip(*aligned))
keys = cols[:2]
vals = cols[2:]
elided_cols = it.chain(
# max width reduces for the indent, plus 2 for the closing "]," on each line
_elide_zip_table(aligned, widths, max_width=(max_width or inf) - tabstop - 2),
keys,
_elide_zip_table(
vals,
len(aligned),
widths[2:],
# max width reduces for the indent, minus 2 for the closing "]," minus the
# width of the key and opening brace
max_width=(max_width or inf) - tabstop - 2 - sum(widths[:2]),
),
[["],\n"] * len(table)],
)
return "".join(
Expand All @@ -45,7 +58,10 @@ def _format_zip_row(key: str, row: list[str]) -> list[str]:
quote_wrap(val) + sep
for val, sep in it.zip_longest(row, it.repeat(", ", len(row) - 1), fillvalue="")
]
return [f"{quote_wrap(key)}: "] + ["[" + formatted_values[0]] + formatted_values[1:]
row = [f"{quote_wrap(key)}: "]
row.append("[")
row.extend(formatted_values)
return row


def _align_zip_table(table: list[list[str]], widths: list[int]) -> list[list[str]]:
Expand All @@ -57,22 +73,19 @@ def _align_zip_table(table: list[list[str]], widths: list[int]) -> list[list[str


def _elide_zip_table(
table: list[list[str]], widths: list[int], max_width: int | float
) -> list[list[str]] | list[tuple[str]] | list[list[str] | tuple[str]]:
cols: list[Sequence[str]], col_len: int, widths: list[int], max_width: int | float
) -> list[Sequence[str]]:
def new_col(val: str):
return [[val] * len(table)]
return [[val] * col_len]

overflow = int(max(sum(widths) - (max_width or inf), 0))
overflow = int(max(sum(widths) - max_width, 0))
elision = _find_elision(list(widths), slice(0, 0), overflow)
cols = list(zip(*table))
if elision != slice(0, 0):
return list(
it.chain(
cols[: elision.start]
if elision.start > 1
else [cols[0], *new_col("[")],
cols[: elision.start],
new_col("..."),
(new_col(" ") if elision.stop - elision.start < len(cols) - 1 else []),
(new_col(" ") if elision.stop - elision.start < len(cols) else []),
cols[elision.stop :],
)
)
Expand All @@ -85,12 +98,11 @@ def _find_elision(widths: list[int], excluded: slice, overflow: int) -> slice:
return excluded
span = excluded.stop - excluded.start
# don't let span become longer than list of values
if span >= len(widths) - 1:
if span >= len(widths):
return excluded

# subtract 1 to account for the key, then add it back when getting the mid index
num_vals = len(widths) - 1
mid = floor(num_vals / 2) + 1
num_vals = len(widths)
mid = floor(num_vals / 2)

# need different rules for handling exclusions of even length depending on whether
# theres an even or odd total number of values.
Expand Down
7 changes: 6 additions & 1 deletion snakebids/tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,15 @@ def get_zip_list(
dict[str, list[str]]
zip_list representation of entity-value combinations
"""

def strlist() -> list[str]:
return list()

lists: Iterable[Sequence[str]] = list(zip(*combinations)) or itx.repeatfunc(strlist)
return MultiSelectDict(
{
BidsEntity(str(entity)).wildcard: list(combs)
for entity, combs in zip(entities, zip(*combinations))
for entity, combs in zip(entities, lists)
}
)

Expand Down
2 changes: 1 addition & 1 deletion snakebids/tests/strategies.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def zip_lists(
unique=unique,
)
)
if cull
if cull and len(combinations)
else combinations
)
return helpers.get_zip_list(values, used_combinations)
Expand Down
53 changes: 46 additions & 7 deletions snakebids/tests/test_printing.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,34 @@ def test_ellipses_appears_when_maxwidth_too_short(zip_list: ZipList):
assert "ellipse" in parsed[0]


@given(zip_list=sb_st.zip_lists(max_entities=1, restrict_patterns=True))
def test_no_ellipses_when_no_max_width(zip_list: ZipList):
parsed = zip_list_parser().parse_string(format_zip_lists(zip_list, tabstop=0))
assert "ellipse" not in parsed[0]


@given(zip_list=sb_st.zip_lists(max_entities=1, restrict_patterns=True))
def test_no_ellipses_when_max_width_long_enouth(zip_list: ZipList):
width = len(format_zip_lists(zip_list, tabstop=0).splitlines()[1])
parsed = zip_list_parser().parse_string(
format_zip_lists(zip_list, width, tabstop=0)
)
assert "ellipse" not in parsed[0]


@given(
zip_list=sb_st.zip_lists(
max_entities=1, min_values=0, max_values=0, restrict_patterns=True
)
)
def test_no_ellipses_appears_when_ziplist_empty(zip_list: ZipList):
width = len(format_zip_lists(zip_list, tabstop=0).splitlines()[1])
parsed = zip_list_parser().parse_string(
format_zip_lists(zip_list, width - 1, tabstop=0)
)
assert "ellipse" not in parsed[0]


@given(
zip_list=sb_st.zip_lists(
min_values=1, max_values=4, max_entities=4, restrict_patterns=True
Expand Down Expand Up @@ -63,7 +91,9 @@ def test_values_balanced_around_elision_correctly(zip_list: ZipList, width: int)

class TestCorrectNumberOfLinesCreated:
@given(
zip_list=sb_st.zip_lists(max_values=1, max_entities=6, restrict_patterns=True),
zip_list=sb_st.zip_lists(
min_values=0, max_values=1, max_entities=6, restrict_patterns=True
),
)
def test_in_zip_list(self, zip_list: ZipList):
assert (
Expand All @@ -72,7 +102,7 @@ def test_in_zip_list(self, zip_list: ZipList):

@given(
component=sb_st.bids_components(
max_values=1, max_entities=6, restrict_patterns=True
min_values=0, max_values=1, max_entities=6, restrict_patterns=True
),
)
def test_in_component(self, component: BidsComponent):
Expand All @@ -91,11 +121,13 @@ def test_in_dataset(self, dataset: BidsDataset):


class TestIsValidPython:
@given(zip_list=sb_st.zip_lists(restrict_patterns=True))
@given(
zip_list=sb_st.zip_lists(restrict_patterns=True, min_values=0, min_entities=0)
)
def test_in_zip_list(self, zip_list: ZipList):
assert eval(format_zip_lists(zip_list, inf)) == zip_list

@given(component=sb_st.bids_components(restrict_patterns=True))
@given(component=sb_st.bids_components(restrict_patterns=True, min_values=0))
def test_in_component(self, component: BidsComponent):
assert eval(component.pformat(inf)) == component

Expand Down Expand Up @@ -125,13 +157,18 @@ def get_indent_length(line: str):


class TestIndentLengthMultipleOfTabStop:
@given(zip_list=sb_st.zip_lists(restrict_patterns=True), tabstop=st.integers(1, 10))
@given(
zip_list=sb_st.zip_lists(restrict_patterns=True, min_values=0),
tabstop=st.integers(1, 10),
)
def test_in_zip_list(self, zip_list: ZipList, tabstop: int):
for line in format_zip_lists(zip_list, tabstop=tabstop).splitlines():
assert get_indent_length(line) / tabstop in {0, 1}

@given(
component=sb_st.bids_components(max_values=1, restrict_patterns=True),
component=sb_st.bids_components(
min_values=0, max_values=1, restrict_patterns=True
),
tabstop=st.integers(1, 10),
)
def test_in_component(self, component: BidsComponent, tabstop: int):
Expand All @@ -146,7 +183,9 @@ def test_in_dataset(self, dataset: BidsDataset, tabstop: int):

class TestMultipleLevelsOfIndentationUsed:
@given(
component=sb_st.bids_components(max_values=1, restrict_patterns=True),
component=sb_st.bids_components(
min_values=0, max_values=1, restrict_patterns=True
),
tabstop=st.integers(1, 10),
)
def test_in_component(self, component: BidsComponent, tabstop: int):
Expand Down

0 comments on commit b8f7898

Please sign in to comment.