From 76c5d3a0e5509e6047c56593da1f7f7199e23c45 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:04:09 +0000 Subject: [PATCH 01/16] optimized layout_resolve added tests --- src/textual/_layout_resolve.py | 49 ++++++++++++++------------ tests/test_layout_resolve.py | 64 ++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 tests/test_layout_resolve.py diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index e8530c3ada..abadd2a480 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -2,7 +2,7 @@ import sys from fractions import Fraction -from typing import cast, List, Optional, Sequence +from typing import cast, Sequence if sys.version_info >= (3, 8): from typing import Protocol @@ -13,12 +13,12 @@ class Edge(Protocol): """Any object that defines an edge (such as Layout).""" - size: Optional[int] = None + size: int | None fraction: int = 1 min_size: int = 1 -def layout_resolve(total: int, edges: Sequence[Edge]) -> List[int]: +def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: """Divide total space to satisfy size, fraction, and min_size, constraints. The returned list of integers should add up to total in most cases, unless it is @@ -37,33 +37,37 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> List[int]: # Size of edge or None for yet to be determined sizes = [(edge.size or None) for edge in edges] - _Fraction = Fraction + if None not in sizes: + return cast(list[int], sizes) - # While any edges haven't been calculated - while None in sizes: - # Get flexible edges and index to map these back on to sizes list - flexible_edges = [ - (index, edge) - for index, (size, edge) in enumerate(zip(sizes, edges)) - if size is None + # Get flexible edges and index to map these back on to sizes list + flexible_edges = [ + (index, edge) + for index, (size, edge) in enumerate(zip(sizes, edges)) + if size is None + ] + # Remaining space in total + remaining = total - sum(size or 0 for size in sizes) + if remaining <= 0: + # No room for flexible edges + return [ + ((edge.min_size or 1) if size is None else size) + for size, edge in zip(sizes, edges) ] - # Remaining space in total - remaining = total - sum(size or 0 for size in sizes) - if remaining <= 0: - # No room for flexible edges - return [ - ((edge.min_size or 1) if size is None else size) - for size, edge in zip(sizes, edges) - ] + + _Fraction = Fraction + while None in sizes: # Calculate number of characters in a ratio portion portion = _Fraction( remaining, sum((edge.fraction or 1) for _, edge in flexible_edges) ) # If any edges will be less than their minimum, replace size with the minimum - for index, edge in flexible_edges: + for flexible_index, (index, edge) in enumerate(flexible_edges): if portion * edge.fraction <= edge.min_size: sizes[index] = edge.min_size + remaining -= edge.min_size + del flexible_edges[flexible_index] # New fixed size will invalidate calculations, so we need to repeat the process break else: @@ -72,9 +76,8 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> List[int]: # to the following line remainder = _Fraction(0) for index, edge in flexible_edges: - size, remainder = divmod(portion * edge.fraction + remainder, 1) - sizes[index] = size + sizes[index], remainder = divmod(portion * edge.fraction + remainder, 1) break # Sizes now contains integers only - return cast(List[int], sizes) + return cast(list[int], sizes) diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py new file mode 100644 index 0000000000..ab29f69bf6 --- /dev/null +++ b/tests/test_layout_resolve.py @@ -0,0 +1,64 @@ +from typing import NamedTuple + +import pytest +from textual._layout_resolve import layout_resolve + + +class Edge(NamedTuple): + size: int | None = None + fraction: int = 1 + min_size: int = 1 + + +def test_single(): + # One edge fixed size + assert layout_resolve(100, [Edge(10)]) == [10] + # One edge fraction of 1 + assert layout_resolve(100, [Edge(None, 1)]) == [100] + # One edge fraction 3 + assert layout_resolve(100, [Edge(None, 2)]) == [100] + # One edge, fraction1, min size 20 + assert layout_resolve(100, [Edge(None, 1, 20)]) == [100] + # One edge fraction 1, min size 120 + assert layout_resolve(100, [Edge(None, 1, 120)]) == [120] + + +def test_two(): + # Two edges fixed size + assert layout_resolve(100, [Edge(10), Edge(20)]) == [10, 20] + # Two edges, fraction 1 each + assert layout_resolve(100, [Edge(None, 1), Edge(None, 1)]) == [50, 50] + # Two edges, one with fraction 2, one with fraction 1 + # Note first value is rounded down, second is rounded up + assert layout_resolve(100, [Edge(None, 2), Edge(None, 1)]) == [66, 34] + # Two edges, both with fraction 2 + assert layout_resolve(100, [Edge(None, 2), Edge(None, 2)]) == [50, 50] + # Two edges, one with fraction 3, one with fraction 1 + assert layout_resolve(100, [Edge(None, 3), Edge(None, 1)]) == [75, 25] + # Two edges, one with fraction 3, one with fraction 1, second with min size of 30 + assert layout_resolve(100, [Edge(None, 3), Edge(None, 1, 30)]) == [70, 30] + # Two edges, one with fraction 1 and min size 30, one with fraction 3 + assert layout_resolve(100, [Edge(None, 1, 30), Edge(None, 3)]) == [30, 70] + + +@pytest.mark.parametrize( + "size, edges, result", + [ + (10, [Edge(None, 1), Edge(None, 1), Edge(None, 1)], [3, 3, 4]), + (10, [Edge(5), Edge(None, 1), Edge(None, 1)], [5, 2, 3]), + (10, [Edge(None, 2), Edge(None, 1), Edge(None, 1)], [5, 2, 3]), + (10, [Edge(None, 2), Edge(3), Edge(None, 1)], [4, 3, 3]), + ( + 10, + [Edge(None, 2), Edge(None, 1), Edge(None, 1), Edge(None, 1)], + [4, 2, 2, 2], + ), + ( + 10, + [Edge(None, 4), Edge(None, 1), Edge(None, 1), Edge(None, 1)], + [5, 2, 1, 2], + ), + ], +) +def test_multiple(size, edges, result): + assert layout_resolve(size, edges) == result From ca009d88b234793d428e349c9f74839f5e825ad7 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:07:01 +0000 Subject: [PATCH 02/16] removed edge defaults --- src/textual/_layout_resolve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index abadd2a480..417566f49b 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -14,8 +14,8 @@ class Edge(Protocol): """Any object that defines an edge (such as Layout).""" size: int | None - fraction: int = 1 - min_size: int = 1 + fraction: int + min_size: int def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: From 4224f59b9b3598bc82203515f8ed91b8833a52bb Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:12:40 +0000 Subject: [PATCH 03/16] allow tests to use new annotations --- tests/test_layout_resolve.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py index ab29f69bf6..29c2926aa0 100644 --- a/tests/test_layout_resolve.py +++ b/tests/test_layout_resolve.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from typing import NamedTuple import pytest From 68d9a66f6cb4e23a064c519630d01555cf26aadb Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:15:32 +0000 Subject: [PATCH 04/16] stringify cast --- src/textual/_layout_resolve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 417566f49b..c0d72a16b1 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -38,7 +38,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: sizes = [(edge.size or None) for edge in edges] if None not in sizes: - return cast(list[int], sizes) + return cast("list[int]", sizes) # Get flexible edges and index to map these back on to sizes list flexible_edges = [ @@ -80,4 +80,4 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: break # Sizes now contains integers only - return cast(list[int], sizes) + return cast("list[int]", sizes) From c2e327cee38073fa3a5fa2cc270ada733b4ae52b Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:30:06 +0000 Subject: [PATCH 05/16] simplify check, add test --- src/textual/_layout_resolve.py | 2 +- tests/test_layout_resolve.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index c0d72a16b1..af45b20324 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -56,7 +56,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: ] _Fraction = Fraction - while None in sizes: + while flexible_edges: # Calculate number of characters in a ratio portion portion = _Fraction( remaining, sum((edge.fraction or 1) for _, edge in flexible_edges) diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py index 29c2926aa0..03a503ca2d 100644 --- a/tests/test_layout_resolve.py +++ b/tests/test_layout_resolve.py @@ -60,6 +60,7 @@ def test_two(): [Edge(None, 4), Edge(None, 1), Edge(None, 1), Edge(None, 1)], [5, 2, 1, 2], ), + (2, [Edge(None, 1), Edge(None, 1), Edge(None, 1)], [1, 1, 1]), ], ) def test_multiple(size, edges, result): From d628b30845dab6bab934c656cb14883a60ad566f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:38:03 +0000 Subject: [PATCH 06/16] extra test --- tests/test_layout_resolve.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py index 03a503ca2d..f187d8f1df 100644 --- a/tests/test_layout_resolve.py +++ b/tests/test_layout_resolve.py @@ -28,6 +28,8 @@ def test_single(): def test_two(): # Two edges fixed size assert layout_resolve(100, [Edge(10), Edge(20)]) == [10, 20] + # Two edges, fixed size of one exceeds total + assert layout_resolve(100, [Edge(120), Edge(None, 1)]) == [120, 1] # Two edges, fraction 1 each assert layout_resolve(100, [Edge(None, 1), Edge(None, 1)]) == [50, 50] # Two edges, one with fraction 2, one with fraction 1 @@ -46,6 +48,7 @@ def test_two(): @pytest.mark.parametrize( "size, edges, result", [ + (10, [Edge(8), Edge(None, 0, 2), Edge(4)], [8, 2, 4]), (10, [Edge(None, 1), Edge(None, 1), Edge(None, 1)], [3, 3, 4]), (10, [Edge(5), Edge(None, 1), Edge(None, 1)], [5, 2, 3]), (10, [Edge(None, 2), Edge(None, 1), Edge(None, 1)], [5, 2, 3]), From 1b50d77aaf2ed01f2d1cf7ad92c1d462d3618b45 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Fri, 18 Feb 2022 22:42:02 +0000 Subject: [PATCH 07/16] comments --- src/textual/_layout_resolve.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index af45b20324..a896616a65 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -13,8 +13,11 @@ class Edge(Protocol): """Any object that defines an edge (such as Layout).""" + # Size of edge in cells, or None for no fixed size size: int | None + # Portion of flexible space to use if size is None fraction: int + # Minimim size for edge, in cells min_size: int From 06485495d475fb1f93b0aa20aee3082f4c9d1c3a Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 19 Feb 2022 11:39:11 +0000 Subject: [PATCH 08/16] optimization, more tests --- src/textual/_layout_resolve.py | 6 +++--- tests/test_layout_resolve.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index a896616a65..5a4f08435e 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -58,18 +58,18 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: for size, edge in zip(sizes, edges) ] + total_flexible = sum((edge.fraction or 1) for _, edge in flexible_edges) _Fraction = Fraction while flexible_edges: # Calculate number of characters in a ratio portion - portion = _Fraction( - remaining, sum((edge.fraction or 1) for _, edge in flexible_edges) - ) + portion = _Fraction(remaining, total_flexible) # If any edges will be less than their minimum, replace size with the minimum for flexible_index, (index, edge) in enumerate(flexible_edges): if portion * edge.fraction <= edge.min_size: sizes[index] = edge.min_size remaining -= edge.min_size + total_flexible -= edge.fraction del flexible_edges[flexible_index] # New fixed size will invalidate calculations, so we need to repeat the process break diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py index f187d8f1df..9f44bcc5c8 100644 --- a/tests/test_layout_resolve.py +++ b/tests/test_layout_resolve.py @@ -64,6 +64,28 @@ def test_two(): [5, 2, 1, 2], ), (2, [Edge(None, 1), Edge(None, 1), Edge(None, 1)], [1, 1, 1]), + ( + 2, + [ + Edge(None, 1, min_size=5), + Edge(None, 1, min_size=4), + Edge(None, 1, min_size=3), + ], + [5, 4, 3], + ), + ( + 18, + [ + Edge(None, 1, min_size=1), + Edge(3), + Edge(None, 1, min_size=1), + Edge(4), + Edge(None, 1, min_size=1), + Edge(5), + Edge(None, 1, min_size=1), + ], + [1, 3, 2, 4, 1, 5, 2], + ), ], ) def test_multiple(size, edges, result): From 3bcabcf32f862b0d9d794d17c1fb2e8997c63ea7 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 19 Feb 2022 11:44:54 +0000 Subject: [PATCH 09/16] fix --- src/textual/_layout_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 5a4f08435e..252bfd2a6f 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -69,7 +69,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: if portion * edge.fraction <= edge.min_size: sizes[index] = edge.min_size remaining -= edge.min_size - total_flexible -= edge.fraction + total_flexible -= edge.fraction or 1 del flexible_edges[flexible_index] # New fixed size will invalidate calculations, so we need to repeat the process break From 61dc1494d55651c43285a980c192e2612ce44f43 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 19 Feb 2022 12:06:14 +0000 Subject: [PATCH 10/16] remote micro-optimization --- src/textual/_layout_resolve.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 252bfd2a6f..518362d97f 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -59,10 +59,9 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: ] total_flexible = sum((edge.fraction or 1) for _, edge in flexible_edges) - _Fraction = Fraction while flexible_edges: # Calculate number of characters in a ratio portion - portion = _Fraction(remaining, total_flexible) + portion = Fraction(remaining, total_flexible) # If any edges will be less than their minimum, replace size with the minimum for flexible_index, (index, edge) in enumerate(flexible_edges): @@ -77,7 +76,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: # Distribute flexible space and compensate for rounding error # Since edge sizes can only be integers we need to add the remainder # to the following line - remainder = _Fraction(0) + remainder = Fraction(0) for index, edge in flexible_edges: sizes[index], remainder = divmod(portion * edge.fraction + remainder, 1) break From dfa77d27448325622e2ad1b0ade83f0df964055e Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 21 Feb 2022 10:22:46 +0000 Subject: [PATCH 11/16] comments --- src/textual/_layout_resolve.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 518362d97f..26343949d2 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -41,6 +41,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: sizes = [(edge.size or None) for edge in edges] if None not in sizes: + # No flexible edges return cast("list[int]", sizes) # Get flexible edges and index to map these back on to sizes list @@ -58,6 +59,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: for size, edge in zip(sizes, edges) ] + # Get the total fraction value for all flexible edges total_flexible = sum((edge.fraction or 1) for _, edge in flexible_edges) while flexible_edges: # Calculate number of characters in a ratio portion @@ -65,7 +67,9 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: # If any edges will be less than their minimum, replace size with the minimum for flexible_index, (index, edge) in enumerate(flexible_edges): - if portion * edge.fraction <= edge.min_size: + if portion * edge.fraction < edge.min_size: + # This flexible edge will be smaller than its minimum size + # We need to fix the size and redistribute the outstanding space sizes[index] = edge.min_size remaining -= edge.min_size total_flexible -= edge.fraction or 1 From d4c12581150f846ac0128185fbc23319d419ae15 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 21 Feb 2022 10:49:09 +0000 Subject: [PATCH 12/16] list expressions are faster --- src/textual/_layout_resolve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 26343949d2..fd8268f0e7 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -51,7 +51,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: if size is None ] # Remaining space in total - remaining = total - sum(size or 0 for size in sizes) + remaining = total - sum([size or 0 for size in sizes]) if remaining <= 0: # No room for flexible edges return [ @@ -60,7 +60,7 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: ] # Get the total fraction value for all flexible edges - total_flexible = sum((edge.fraction or 1) for _, edge in flexible_edges) + total_flexible = sum([(edge.fraction or 1) for _, edge in flexible_edges]) while flexible_edges: # Calculate number of characters in a ratio portion portion = Fraction(remaining, total_flexible) From 140ff006bac956ff04d41ee0bb5d277875a64fd6 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 21 Feb 2022 11:03:08 +0000 Subject: [PATCH 13/16] docstrings --- src/textual/_layout_resolve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index fd8268f0e7..6fd34e20a9 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -32,10 +32,10 @@ def layout_resolve(total: int, edges: Sequence[Edge]) -> list[int]: Args: total (int): Total number of characters. - edges (List[Edge]): Edges within total space. + edges (Sequence[Edge]): Edges within total space. Returns: - List[int]: Number of characters for each edge. + list[int]: Number of characters for each edge. """ # Size of edge or None for yet to be determined sizes = [(edge.size or None) for edge in edges] From b23edf231a4cb617e9020c5c23d86275111f981f Mon Sep 17 00:00:00 2001 From: Darren Burns Date: Mon, 21 Feb 2022 11:40:42 +0000 Subject: [PATCH 14/16] Testing layout resolve zero width, and empty list cases --- tests/test_layout_resolve.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/test_layout_resolve.py b/tests/test_layout_resolve.py index 9f44bcc5c8..77ba2643ef 100644 --- a/tests/test_layout_resolve.py +++ b/tests/test_layout_resolve.py @@ -12,6 +12,14 @@ class Edge(NamedTuple): min_size: int = 1 +def test_empty(): + assert layout_resolve(10, []) == [] + + +def test_total_zero(): + assert layout_resolve(0, [Edge(10)]) == [10] + + def test_single(): # One edge fixed size assert layout_resolve(100, [Edge(10)]) == [10] From 78f8a3ed9e1d0d2f87b46530a49baa228211b00d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 21 Feb 2022 11:44:25 +0000 Subject: [PATCH 15/16] Update src/textual/_layout_resolve.py Co-authored-by: Darren Burns --- src/textual/_layout_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 6fd34e20a9..04697d7dd2 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -17,7 +17,7 @@ class Edge(Protocol): size: int | None # Portion of flexible space to use if size is None fraction: int - # Minimim size for edge, in cells + # Minimum size for edge, in cells min_size: int From 4638541aede231745c459010430b8dcb6cfc389d Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Mon, 21 Feb 2022 13:34:20 +0000 Subject: [PATCH 16/16] Change tuple to dataclass --- src/textual/_layout_resolve.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/textual/_layout_resolve.py b/src/textual/_layout_resolve.py index 98d9dafe3b..71f36e4946 100644 --- a/src/textual/_layout_resolve.py +++ b/src/textual/_layout_resolve.py @@ -24,11 +24,11 @@ class EdgeProtocol(Protocol): @dataclass -class Edge(NamedTuple): +class Edge: size: int | None = None fraction: int | None = 1 min_size: int = 1 - + def layout_resolve(total: int, edges: Sequence[EdgeProtocol]) -> List[int]: """Divide total space to satisfy size, fraction, and min_size, constraints.