From 089dce1b41c641b111e68a3dcb5bdfa6a0ea3dc1 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:39:12 +0100 Subject: [PATCH 1/7] Add a unit test for #2502 Currently marked as xfail, but this gets it down to the most basic level. --- tests/test_resolve.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index c36e1a406..e6799d20c 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -1,4 +1,5 @@ from fractions import Fraction +from itertools import chain import pytest @@ -122,3 +123,30 @@ def test_resolve_fraction_unit(): Fraction(32), resolve_dimension="width", ) == Fraction(2) + + +@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2502") +def test_resolve_issue_2502(): + """Test https://github.com/Textualize/textual/issues/2502""" + + widgets_fr = [Widget() for _ in range(50)] + widgets_min_width = [Widget() for _ in range(50)] + + for widget in widgets_fr: + widget.styles.width = "1fr" + widget.styles.min_width = 3 + + for widget in widgets_min_width: + widget.styles.width = 0 + + styles = ( + widget.styles + for widget in chain.from_iterable(zip(widgets_fr, widgets_min_width)) + ) + + assert isinstance( + resolve_fraction_unit( + styles, Size(80, 24), Size(80, 24), Fraction(10), resolve_dimension="width" + ), + Fraction, + ) From 052ec83b7aa25160a5ae8b70d3b765d7f32ae6bf Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:44:30 +0100 Subject: [PATCH 2/7] Make the test as small as possible --- tests/test_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index e6799d20c..a0fde4cd4 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -134,7 +134,7 @@ def test_resolve_issue_2502(): for widget in widgets_fr: widget.styles.width = "1fr" - widget.styles.min_width = 3 + widget.styles.min_width = 1 for widget in widgets_min_width: widget.styles.width = 0 From a5cc96cbc7750a6d41aeaad641b76bc25beadd35 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 09:44:51 +0100 Subject: [PATCH 3/7] Make a pass of the #2502 test a fail If/when I get this actually passing, I want the test to appear to fail so I know things have changed for the better. This makes sense, trust me. --- tests/test_resolve.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index a0fde4cd4..c7cdd485e 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -125,7 +125,9 @@ def test_resolve_fraction_unit(): ) == Fraction(2) -@pytest.mark.xfail(reason="https://github.com/Textualize/textual/issues/2502") +@pytest.mark.xfail( + strict=True, reason="https://github.com/Textualize/textual/issues/2502" +) def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" From a77dbf4beefc2d0279729af28084c3b77c77fec5 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 10:38:09 +0100 Subject: [PATCH 4/7] Tentative fix for resolve_fraction_unit ZeroDivision error I'll admit to not really following what the code does, so will really need someone with a better understanding of the aim of this code to look over the proposed fix; but based on a bunch of runs and hand-debugging, this seems to do the job. This passes all existing tests and also removes the reported error. On the other hand I'm not confident that I'm *not* just masking an underlying issue with this function. --- src/textual/_resolve.py | 4 ++-- tests/test_resolve.py | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index ddbdd1d5c..1ee40af9e 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -143,7 +143,7 @@ def resolve_fraction_unit( resolved: list[Fraction | None] = [None] * len(resolve) remaining_fraction = Fraction(sum(scalar.value for scalar, _, _ in resolve)) - while True: + while remaining_fraction: remaining_space_changed = False resolve_fraction = Fraction(remaining_space, remaining_fraction) for index, (scalar, min_value, max_value) in enumerate(resolve): @@ -166,7 +166,7 @@ def resolve_fraction_unit( return ( Fraction(remaining_space, remaining_fraction) - if remaining_space + if remaining_space > 0 else Fraction(1) ) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index c7cdd485e..b73db6c55 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -125,9 +125,6 @@ def test_resolve_fraction_unit(): ) == Fraction(2) -@pytest.mark.xfail( - strict=True, reason="https://github.com/Textualize/textual/issues/2502" -) def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" From ee707130023207b8b1e969c633228d9481d7dbce Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 13:35:55 +0100 Subject: [PATCH 5/7] Simplify the resolver zero division bug unit test --- tests/test_resolve.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/tests/test_resolve.py b/tests/test_resolve.py index b73db6c55..202299747 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -128,24 +128,17 @@ def test_resolve_fraction_unit(): def test_resolve_issue_2502(): """Test https://github.com/Textualize/textual/issues/2502""" - widgets_fr = [Widget() for _ in range(50)] - widgets_min_width = [Widget() for _ in range(50)] - - for widget in widgets_fr: - widget.styles.width = "1fr" - widget.styles.min_width = 1 - - for widget in widgets_min_width: - widget.styles.width = 0 - - styles = ( - widget.styles - for widget in chain.from_iterable(zip(widgets_fr, widgets_min_width)) - ) + widget = Widget() + widget.styles.width = "1fr" + widget.styles.min_width = 11 assert isinstance( resolve_fraction_unit( - styles, Size(80, 24), Size(80, 24), Fraction(10), resolve_dimension="width" + (widget.styles,), + Size(80, 24), + Size(80, 24), + Fraction(10), + resolve_dimension="width", ), Fraction, ) From 8d7ae4d1fbd6dc74562819036d4b3f0b3d822835 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 13:36:32 +0100 Subject: [PATCH 6/7] Ensure that remaining fraction is always above zero --- src/textual/_resolve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/textual/_resolve.py b/src/textual/_resolve.py index 1ee40af9e..79a1a1e3e 100644 --- a/src/textual/_resolve.py +++ b/src/textual/_resolve.py @@ -143,7 +143,7 @@ def resolve_fraction_unit( resolved: list[Fraction | None] = [None] * len(resolve) remaining_fraction = Fraction(sum(scalar.value for scalar, _, _ in resolve)) - while remaining_fraction: + while remaining_fraction > 0: remaining_space_changed = False resolve_fraction = Fraction(remaining_space, remaining_fraction) for index, (scalar, min_value, max_value) in enumerate(resolve): From 0eeadf9ae9136eedb830a17bbe6103b717e89495 Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Tue, 9 May 2023 21:01:31 +0100 Subject: [PATCH 7/7] Update the CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa2923fac..2e65e54e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](http://keepachangelog.com/) and this project adheres to [Semantic Versioning](http://semver.org/). +## Unreleased + +### Fixed + +- Fixed `ZeroDivisionError` in `resolve_fraction_unit` https://github.com/Textualize/textual/issues/2502 + ## [0.24.1] - 2023-05-08 ### Fixed