From 3a849e262cbfb6e5d119f10d357ad5d497773be5 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 4 Mar 2024 11:57:12 +0000 Subject: [PATCH 01/43] Add custom scoring capabilities to Target class Refactor max and min score methods to key from external data dictionary Add alternative constructor to build a target directly from a face specification --- archeryutils/targets.py | 227 ++++++++++++++++++++++------- archeryutils/tests/test_rounds.py | 10 ++ archeryutils/tests/test_targets.py | 119 ++++++++++++++- 3 files changed, 300 insertions(+), 56 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 49602fa..188bdf0 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -1,6 +1,7 @@ """Module for representing a Target for archery applications.""" -from typing import Literal, Union, get_args +from functools import partial +from typing import Literal, NamedTuple, Union, get_args from archeryutils.constants import Length @@ -18,8 +19,37 @@ "Beiter_hit_miss", "Worcester", "Worcester_2_ring", + "Custom", ] +# TypeAlias (annotate explicitly in py3.10+) +FaceSpec = dict[float, int] + + +class Rings(NamedTuple): + """Container for data on max and min scores on target face types.""" + + high: int + low: int + + +_scoring_system_data = { + "5_zone": Rings(high=9, low=1), + "10_zone": Rings(high=10, low=1), + "10_zone_compound": Rings(high=10, low=1), + "10_zone_6_ring": Rings(high=10, low=5), + "10_zone_5_ring": Rings(high=10, low=6), + "10_zone_5_ring_compound": Rings(high=10, low=6), + "WA_field": Rings(high=6, low=1), + "IFAA_field": Rings(high=5, low=3), + "IFAA_field_expert": Rings(high=5, low=1), + "Beiter_hit_miss": Rings(high=1, low=1), + "Worcester": Rings(high=5, low=1), + "Worcester_2_ring": Rings(high=5, low=4), +} + +_rnd6 = partial(round, ndigits=6) + class Target: """ @@ -84,6 +114,11 @@ class Target: """ + _face_spec: FaceSpec + supported_systems = get_args(ScoringSystem) + supported_distance_units = Length.yard | Length.metre + supported_diameter_units = Length.cm | Length.inch | Length.metre + def __init__( self, scoring_system: ScoringSystem, @@ -91,21 +126,20 @@ def __init__( distance: Union[float, tuple[float, str]], indoor: bool = False, ) -> None: - systems = get_args(ScoringSystem) - if scoring_system not in systems: + if scoring_system not in self.supported_systems: msg = ( f"""Invalid Target Face Type specified.\n""" - f"""Please select from '{"', '".join(systems)}'.""" + f"""Please select from '{"', '".join(self.supported_systems)}'.""" ) - raise ValueError(msg) if isinstance(distance, tuple): (distance, native_dist_unit) = distance else: native_dist_unit = "metre" - if native_dist_unit not in Length.yard | Length.metre: + + if native_dist_unit not in self.supported_distance_units: msg = ( f"Distance unit '{native_dist_unit}' not recognised. " "Select from 'yard' or 'metre'." @@ -117,7 +151,7 @@ def __init__( (diameter, native_diameter_unit) = diameter else: native_diameter_unit = "cm" - if native_diameter_unit not in Length.cm | Length.inch | Length.metre: + if native_diameter_unit not in self.supported_diameter_units: msg = ( f"Diameter unit '{native_diameter_unit}' not recognised. " "Select from 'cm', 'inch' or 'metre'" @@ -132,6 +166,66 @@ def __init__( self.native_dist_unit = Length.definitive_unit(native_dist_unit) self.indoor = indoor + @classmethod + def from_spec( + cls, + face_spec: Union[FaceSpec, tuple[FaceSpec, str]], + diameter: Union[float, tuple[float, str]], + distance: Union[float, tuple[float, str]], + indoor: bool = False, + ) -> "Target": + """ + Constuctor to build a target with custom scoring system. + + Optionally can convert units at the time of construction. + Diameter must still be provided as a seperate arguement as it is impossible + to know what the nominal diameter would be from the face specification + without a known scoring system. However it is superceeded by face_spec + and has no effect when calculating handicaps. + + Parameters + ---------- + face_spec : dict of floats to ints or 2-tuple of dict, str + Target face specification, a mapping of target ring sizes to score. + Default units are assumed as [metres] but can be provided as the second + element of a tuple. + diameter : float or tuple of float, str + Target face diameter (and units, default [cm]) + distance : float or tuple of float, str + linear distance from archer to target (and units, default [metres]) + indoor : bool + Is target indoors for arrow diameter purposes? default = False + + Returns + ------- + Target + Instance of Target class with scoring system set as "Custom" and + face specification stored. + + Examples + -------- + >>> #WA 18m compound triple spot + >>> specs = {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} + >>> target = Target.from_spec(specs, 40, 18) + """ + if isinstance(face_spec, tuple): + spec_data, spec_units = face_spec + + if spec_units not in cls.supported_diameter_units: + msg = ( + f"Face specification unit '{spec_units}' not recognised. " + "Select from 'cm', 'inch' or 'metre'" + ) + raise ValueError(msg) + face_spec = { + _rnd6(Length.to_metres(ring_diam, spec_units)): score + for ring_diam, score in spec_data.items() + } + + target = cls("Custom", diameter, distance, indoor) + target._face_spec = face_spec # noqa: SLF001 private member access + return target + def __repr__(self) -> str: """Return a representation of a Target instance.""" diam, diamunit = self.native_diameter @@ -148,6 +242,11 @@ def __repr__(self) -> str: def __eq__(self, other: object) -> bool: """Check equality of Targets based on parameters.""" if isinstance(other, Target): + if self.scoring_system == other.scoring_system == "Custom": + return ( + self._face_spec == other._face_spec + and self._parameters() == other._parameters() + ) return self._parameters() == other._parameters() return NotImplemented @@ -162,6 +261,11 @@ def _parameters(self): self.indoor, ) + @property + def is_custom(self): + """Check if this Target uses a custom scoring system.""" + return self.scoring_system == "Custom" + @property def native_distance(self) -> tuple[float, str]: """Get target distance in original native units.""" @@ -198,28 +302,11 @@ def max_score(self) -> float: >>> mytarget.max_score() 10.0 """ - if self.scoring_system in ("5_zone"): - return 9.0 - if self.scoring_system in ( - "10_zone", - "10_zone_compound", - "10_zone_6_ring", - "10_zone_6_ring_compound", - "10_zone_5_ring", - "10_zone_5_ring_compound", - ): - return 10.0 - if self.scoring_system in ("WA_field"): - return 6.0 - if self.scoring_system in ( - "IFAA_field", - "IFAA_field_expert", - "Worcester", - "Worcester_2_ring", - ): - return 5.0 - if self.scoring_system in ("Beiter_hit_miss"): - return 1.0 + if self.is_custom: + return max(self._face_spec.values()) + data = _scoring_system_data.get(self.scoring_system) + if data: + return data.high # NB: Should be hard (but not impossible) to get here without catching earlier. msg = f"Target face '{self.scoring_system}' has no specified maximum score." raise ValueError(msg) @@ -244,32 +331,62 @@ def min_score(self) -> float: >>> mytarget.min_score() 1.0 """ - if self.scoring_system in ( - "5_zone", - "10_zone", - "10_zone_compound", - "WA_field", - "IFAA_field_expert", - "Worcester", - ): - return 1.0 - if self.scoring_system in ( - "10_zone_6_ring", - "10_zone_6_ring_compound", - ): - return 5.0 - if self.scoring_system in ( - "10_zone_5_ring", - "10_zone_5_ring_compound", - ): - return 6.0 - if self.scoring_system in ("Worcester_2_ring",): - return 4.0 - if self.scoring_system in ("IFAA_field",): - return 3.0 - if self.scoring_system in ("Beiter_hit_miss"): - # For Beiter options are hit and miss, so return 0 here - return 0.0 + if self.is_custom: + return min(self._face_spec.values()) + data = _scoring_system_data.get(self.scoring_system) + if data: + return data.low # NB: Should be hard (but not impossible) to get here without catching earlier. msg = f"Target face '{self.scoring_system}' has no specified minimum score." raise ValueError(msg) + + def get_face_spec(self) -> FaceSpec: + # Could replace with mapping face to lambda that returns spec + """Derive specifications for common/supported targets. + + Returns + ------- + spec : dict + Mapping of target ring sizes in [metres] to score + """ + system = self.scoring_system + tar_dia = self.diameter + + if system == "Custom": + return self._face_spec + + data = _scoring_system_data.get(system) + if not data: + # no data for scoring system, raise + msg = f"No rule for calculating scoring for face type {system}." + raise ValueError(msg) + + # calculate missing rings for certain targets from minimum score + missing = data.low - 1 + + if system == "5_zone": + spec = {_rnd6((n + 1) * tar_dia / 10): 10 - n for n in range(1, 11, 2)} + + elif system in ("10_zone", "10_zone_6_ring", "10_zone_5_ring"): + spec = {_rnd6(n * tar_dia / 10): 11 - n for n in range(1, 11 - missing)} + + elif system in ("10_zone_compound", "10_zone_5_ring_compound"): + spec = {_rnd6(tar_dia / 20): 10} | { + _rnd6(n * tar_dia / 10): 11 - n for n in range(2, 11 - missing) + } + + elif system == "WA_field": + spec = {_rnd6(tar_dia / 10): 6} | { + _rnd6(n * tar_dia / 5): 6 - n for n in range(1, 6) + } + + elif system == "IFAA_field": + spec = {_rnd6(n * tar_dia / 5): 5 - n // 2 for n in range(1, 6, 2)} + + elif system == "Beiter_hit_miss": + spec = {tar_dia: 1} + + elif system in ("Worcester", "Worcester_2_ring", "IFAA_field_expert"): + spec = {_rnd6(n * tar_dia / 5): 6 - n for n in range(1, 6 - missing)} + + return spec diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 5540812..5582c98 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -22,6 +22,8 @@ class TestPass: test behaviour of default location test_negative_arrows() test behaviour of negative arrow number + test_custom_constructor() + test construction from a custom target specification test_properties() test setting of Pass properties test_max_score() @@ -118,6 +120,14 @@ def test_properties(self) -> None: assert test_pass.indoor is False assert test_pass.native_diameter_unit == "cm" + def test_custom_target(self) -> None: + """ + Check that pass can be constructed from a custom target specification + """ + target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) + test_pass = Pass(30, target) + assert test_pass.target.is_custom + @pytest.mark.parametrize( "face_type,max_score_expected", [ diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 77894a4..20ed7a0 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -213,7 +213,7 @@ def test_max_score_invalid_face_type(self) -> None: ("IFAA_field_expert", 1), ("Worcester", 1), ("Worcester_2_ring", 4), - ("Beiter_hit_miss", 0), + ("Beiter_hit_miss", 1), ], ) def test_min_score( @@ -236,3 +236,120 @@ def test_min_score_invalid_face_type(self) -> None: # Silence mypy as scoring_system must be a valid literal ScoringSystem target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] target.min_score() + + @pytest.mark.parametrize( + "scoring_system, diam", + [ + ("5_zone", 122), + ("10_zone", 80), + ("WA_field", 80), + ("IFAA_field", 50), + ("Beiter_hit_miss", 6), + ("Worcester", (16, "inch")), + ("10_zone_6_ring", 80), + ("10_zone_5_ring_compound", 40), + ], + ) + def test_get_face_spec(self, scoring_system, diam) -> None: + """ + Check that target returns correct face specifications from supported scoring systems. + """ + expected_spec = { + "5_zone": {0.244: 9, 0.488: 7, 0.732: 5, 0.976: 3, 1.22: 1}, + "10_zone": { + 0.08: 10, + 0.16: 9, + 0.24: 8, + 0.32: 7, + 0.4: 6, + 0.48: 5, + 0.56: 4, + 0.64: 3, + 0.72: 2, + 0.8: 1, + }, + "WA_field": {0.08: 6, 0.16: 5, 0.32: 4, 0.48: 3, 0.64: 2, 0.8: 1}, + "IFAA_field": {0.1: 5, 0.3: 4, 0.5: 3}, + "Beiter_hit_miss": {0.06: 1}, + "Worcester": {0.08128: 5, 0.16256: 4, 0.24384: 3, 0.32512: 2, 0.4064: 1}, + "10_zone_6_ring": {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6, 0.48: 5}, + "10_zone_5_ring_compound": {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6}, + } + + target = Target(scoring_system, diam, 30) + assert target.get_face_spec() == expected_spec[scoring_system] + + +class TestCustomScoringTarget: + """ + Tests for Target class with custom scoring + """ + + _11zone_spec = {0.02: 11, 0.04: 10, 0.8: 9, 0.12: 8, 0.16: 7, 0.2: 6} + + def test_constructor(self) -> None: + """ + Can initialise Target with a custom scoring system and spec + """ + + target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) + assert target.distance == 50.0 * 0.9144 + assert target.diameter == 0.8 + assert target.scoring_system == "Custom" + assert target.get_face_spec() == {0.1: 3, 0.5: 1} + + def test_face_spec_units(self) -> None: + """ + Check custom Target can be constructed with alternative units. + """ + target = Target.from_spec(({10: 5, 20: 4, 30: 3}, "cm"), 50, 30) + assert target.get_face_spec() == {0.1: 5, 0.2: 4, 0.3: 3} + + def test_invalid_face_spec_units(self) -> None: + """ + Check custom Target cannot be constructed with unsupported units. + """ + with pytest.raises( + ValueError, + # match= + ): + Target.from_spec(({10: 5, 20: 4, 30: 3}, "bananas"), 50, 30) + + @pytest.mark.parametrize( + "spec, args, result", + [ + pytest.param( + {0.2: 2, 0.4: 1}, + (40, 20, True), + True, + id="duplicate", + ), + pytest.param( + {0.4: 5}, + (40, 20, True), + False, + id="different_spec", + ), + ], + ) + def test_equality(self, spec, args, result) -> None: + """ + Check custom Target equality comparison is supported. + """ + target = Target.from_spec({0.2: 2, 0.4: 1}, 40, 20, indoor=True) + comparison = target == Target.from_spec(spec, *args) + assert comparison == result + + def test_max_score(self) -> None: + """ + Check that Target with custom scoring system returns correct max score + """ + target = Target.from_spec(self._11zone_spec, 40, 18) + assert target.max_score() == 11 + + def test_min_score(self) -> None: + """ + Check that Target with custom scoring system returns correct min score + """ + target = Target.from_spec(self._11zone_spec, 40, 18) + assert target.min_score() == 6 From 70a510e0dcc6fcdda2d16ff0366cc3bb0ee5e884 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 4 Mar 2024 12:19:14 +0000 Subject: [PATCH 02/43] Clarify Pass.at_target docs for input parameters --- archeryutils/rounds.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 86e7b47..6e9f21b 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -69,16 +69,12 @@ def at_target( # noqa: PLR0913 ``"IFAA_field"`` ``"IFAA_field_expert"`` ``"Beiter_hit_miss"`` ``"Worcester"``\ ``"Worcester_2_ring"``} target face/scoring system type - diameter : float - face diameter in [centimetres] - distance : float - linear distance from archer to target in [metres] - dist_unit : str - The unit distance is measured in. default = 'metres' + diameter : float or tuple of float, str + Target diameter size (and units, default [cm]) + distance : float or tuple of float, str + Target distance (and units, default [metres]) indoor : bool is round indoors for arrow diameter purposes? default = False - diameter_unit : str - The unit face diameter is measured in. default = 'centimetres' Returns ------- From 43c97b459a6cab5e2876be022c993042024491a6 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 4 Mar 2024 12:33:45 +0000 Subject: [PATCH 03/43] Replace implementation of arrow_score with generic calculation from face specification arrow_score no longer dispatches on scoring system and does not have magic numbers --- archeryutils/handicaps/handicap_scheme.py | 144 ++++++++-------------- 1 file changed, 49 insertions(+), 95 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index 2f66bfe..e3615c8 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -31,6 +31,7 @@ """ +import itertools as itr import warnings from abc import ABC, abstractmethod from typing import Optional, TypeVar, Union, overload @@ -42,6 +43,17 @@ FloatArray = TypeVar("FloatArray", float, npt.NDArray[np.float64]) +# itertools.pairwise not available until python 3.10 +# workaround can be removed when support for 3.9 is dropped +# ignore for coverage (runner is > 3.10, ci shows this works on 3.9) +if not hasattr(itr, "pairwise"): # pragma: no cover + + def _pairwise(iterable): + a, b = itr.tee(iterable) + next(b, None) + return zip(a, b) + + itr.pairwise = _pairwise class HandicapScheme(ABC): r""" @@ -162,7 +174,7 @@ def sigma_r(self, handicap: FloatArray, dist: float) -> FloatArray: sig_r = dist * sig_t return sig_r - def arrow_score( # noqa: PLR0912 Too many branches + def arrow_score( self, handicap: FloatArray, target: targets.Target, @@ -216,104 +228,46 @@ def arrow_score( # noqa: PLR0912 Too many branches arw_d = self.arw_d_out arw_rad = arw_d / 2.0 - - tar_dia = target.diameter + spec = target.get_face_spec() sig_r = self.sigma_r(handicap, target.distance) + s_bar = self._s_bar(spec, arw_rad, sig_r) + return s_bar - if target.scoring_system == "5_zone": - s_bar = ( - 9.0 - - 2.0 - * sum( - np.exp(-((((n * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 5) - ) - - np.exp(-((((5.0 * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - ) - - elif target.scoring_system == "10_zone": - s_bar = 10.0 - sum( - np.exp(-((((n * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 11) - ) - - elif target.scoring_system == "10_zone_6_ring": - s_bar = ( - 10.0 - - sum( - np.exp(-((((n * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 6) - ) - - 5.0 * np.exp(-((((6.0 * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - ) - - elif target.scoring_system == "10_zone_compound": - s_bar = ( - 10.0 - - np.exp(-((((tar_dia / 40.0) + arw_rad) / sig_r) ** 2)) - - sum( - np.exp(-((((n * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - for n in range(2, 11) - ) - ) - - elif target.scoring_system == "10_zone_5_ring": - s_bar = ( - 10.0 - - sum( - np.exp(-((((n * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 5) - ) - - 6.0 * np.exp(-((((5.0 * tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - ) - - elif target.scoring_system == "10_zone_5_ring_compound": - s_bar = ( - 10.0 - - np.exp(-((((tar_dia / 40) + arw_rad) / sig_r) ** 2)) - - sum( - np.exp(-((((n * tar_dia / 20) + arw_rad) / sig_r) ** 2)) - for n in range(2, 5) - ) - - 6.0 * np.exp(-((((5 * tar_dia / 20) + arw_rad) / sig_r) ** 2)) - ) - - elif target.scoring_system == "WA_field": - s_bar = ( - 6.0 - - np.exp(-((((tar_dia / 20.0) + arw_rad) / sig_r) ** 2)) - - sum( - np.exp(-((((n * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 6) - ) - ) - - elif target.scoring_system == "IFAA_field": - s_bar = ( - 5.0 - - np.exp(-((((tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - - np.exp(-((((3.0 * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - - 3.0 * np.exp(-((((5.0 * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - ) - - elif target.scoring_system == "Beiter_hit_miss": - s_bar = 1.0 - np.exp(-((((tar_dia / 2.0) + arw_rad) / sig_r) ** 2)) - - elif target.scoring_system in ("Worcester", "IFAA_field_expert"): - s_bar = 5.0 - sum( - np.exp(-((((n * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - for n in range(1, 6) - ) + def _s_bar( + self, target_specs: dict[float, int], arw_rad: float, sig_r: FloatArray + ) -> FloatArray: + """Calculate expected score directly from target ring sizes. - elif target.scoring_system == "Worcester_2_ring": - s_bar = ( - 5.0 - - np.exp(-((((tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - - 4.0 * np.exp(-((((2 * tar_dia / 10.0) + arw_rad) / sig_r) ** 2)) - ) - # No need for else with error as invalid scoring systems handled in Target class + Parameters + ---------- + target_specs : dict[float, int] + Mapping of target ring *diameters* in [metres], to points scored + arw_rad : float + arrow radius in [metres] + sig_r : float + standard deviation of group size [metres] - return s_bar + Returns + ------- + s_bar : float + expected average score per arrow + + Notes + ----- + Assumes that: + - target rings are concentric + - score decreases monotonically as ring sizes increase + """ + target_specs = dict(sorted(target_specs.items())) + ring_sizes = target_specs.keys() + ring_scores = list(itr.chain(target_specs.values(), [0])) + score_drops = (inner - outer for inner, outer in itr.pairwise(ring_scores)) + max_score = max(ring_scores) + + return max_score - sum( + score_drop * np.exp(-(((arw_rad + (ring_diam / 2)) / sig_r) ** 2)) + for ring_diam, score_drop in zip(ring_sizes, score_drops) + ) def score_for_passes( self, From 841b2b766caf0ff57655c04bba57b6cb0cf1de1e Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 4 Mar 2024 12:45:26 +0000 Subject: [PATCH 04/43] Add indirect tests for _s_bar via arrow_score function --- archeryutils/handicaps/handicap_scheme.py | 1 + .../handicaps/tests/test_handicaps.py | 67 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index e3615c8..30cf92e 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -55,6 +55,7 @@ def _pairwise(iterable): itr.pairwise = _pairwise + class HandicapScheme(ABC): r""" Abstract Base Class to represent a generic handicap scheme. diff --git a/archeryutils/handicaps/tests/test_handicaps.py b/archeryutils/handicaps/tests/test_handicaps.py index 800b1fc..6051e7d 100644 --- a/archeryutils/handicaps/tests/test_handicaps.py +++ b/archeryutils/handicaps/tests/test_handicaps.py @@ -73,6 +73,14 @@ Pass.at_target(36, "10_zone", 122, 30, False), ], ) +kings_900_rec = Round( + "Kings 900 (recurve)", + [ + Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), + Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), + Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), + ], +) class TestHandicapScheme: @@ -376,6 +384,51 @@ def test_different_target_faces( assert arrow_score_direct == pytest.approx(arrow_score_expected) + def test_empty_spec(self): + """ + Check expected score is zero when no target rings are defined. + """ + target = Target.from_spec({}, 10, 10) + s_bar = hc.arrow_score(50, target, "AGB") + assert s_bar == 0 + + def test_unsorted_spec(self): + """ + Check expected score is insensitive to order of input spec. + """ + + def _target(spec): + return Target.from_spec(spec, 10, 10) + + s_bar = hc.arrow_score(50, _target({0.1: 1, 0.2: 2, 0.3: 3}), "AA") + s_bar_reversed = hc.arrow_score(50, _target({0.3: 3, 0.2: 2, 0.1: 1}), "AA") + s_bar_unordered = hc.arrow_score(50, _target({0.1: 1, 0.3: 3, 0.2: 2}), "AA") + + assert s_bar_unordered == s_bar_reversed == s_bar + + def test_decimal_ring_scores(self): + """ + Check expected score can be calculated for non integer ring scores + + Uses a target with integer ring scores at twice the value for comparison + """ + target_int = Target.from_spec({0.1: 3, 0.2: 5}, 10, 10) + target_dec = Target.from_spec({0.1: 1.5, 0.2: 2.5}, 10, 10) + + s_bar_int = hc.arrow_score(30, target_int, "AGB") + s_bar_dec = hc.arrow_score(30, target_dec, "AGB") + + assert s_bar_int == 2 * s_bar_dec + + def test_array_handicaps(self): + """ + Check expected score can be calculated for an array of input handicap values + """ + handicaps = np.array([10, 20, 30, 40]) + target = Target.from_spec({0.1: 3, 0.2: 5}, 10, 10) + s_bar = hc.arrow_score(handicaps, target, "AGB") + assert len(s_bar) == len(handicaps) + class TestScoreForPasses: """ @@ -598,6 +651,13 @@ def test_rounded_round_score( hc_sys.score_for_round(20.0, test_round, None, True) == round_score_expected ) + def test_calculation_custom_scoring(self): + """ + Check that score can be calculated for a round with custom scoring + """ + + assert hc.score_for_round(20.0, kings_900_rec, "AGB", None, True) == 896.0 + class TestHandicapFromScore: """ @@ -851,3 +911,10 @@ def test_decimal( ) assert handicap == pytest.approx(handicap_expected) + + def test_calculation_custom_scoring(self): + """ + Check that handicap can be calculated for a round with custom scoring + """ + + assert hc.handicap_from_score(896, kings_900_rec, "AGB", int_prec=True) == 20 From 4915a2849b0ca5c44721e993febe0772a135f53d Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 07:53:47 +0000 Subject: [PATCH 05/43] Linting and formatting to bring up to date with new use of Ruff --- archeryutils/handicaps/handicap_scheme.py | 8 ++-- .../handicaps/tests/test_handicaps.py | 24 +++--------- archeryutils/targets.py | 3 +- archeryutils/tests/test_rounds.py | 4 +- archeryutils/tests/test_targets.py | 37 ++++++------------- 5 files changed, 24 insertions(+), 52 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index 30cf92e..b0f1229 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -53,7 +53,7 @@ def _pairwise(iterable): next(b, None) return zip(a, b) - itr.pairwise = _pairwise + setattr(itr, "pairwise", _pairwise) # noqa: B010 class HandicapScheme(ABC): @@ -107,7 +107,8 @@ def __repr__(self) -> str: @overload @abstractmethod - def sigma_t(self, handicap: float, dist: float) -> float: ... + def sigma_t(self, handicap: float, dist: float) -> float: + ... @overload @abstractmethod @@ -115,7 +116,8 @@ def sigma_t( self, handicap: npt.NDArray[np.float64], dist: float, - ) -> npt.NDArray[np.float64]: ... + ) -> npt.NDArray[np.float64]: + ... @abstractmethod def sigma_t(self, handicap: FloatArray, dist: float) -> FloatArray: diff --git a/archeryutils/handicaps/tests/test_handicaps.py b/archeryutils/handicaps/tests/test_handicaps.py index 6051e7d..98f5110 100644 --- a/archeryutils/handicaps/tests/test_handicaps.py +++ b/archeryutils/handicaps/tests/test_handicaps.py @@ -385,17 +385,13 @@ def test_different_target_faces( assert arrow_score_direct == pytest.approx(arrow_score_expected) def test_empty_spec(self): - """ - Check expected score is zero when no target rings are defined. - """ + """Check expected score is zero when no target rings are defined.""" target = Target.from_spec({}, 10, 10) s_bar = hc.arrow_score(50, target, "AGB") assert s_bar == 0 def test_unsorted_spec(self): - """ - Check expected score is insensitive to order of input spec. - """ + """Check expected score is insensitive to order of input spec.""" def _target(spec): return Target.from_spec(spec, 10, 10) @@ -408,7 +404,7 @@ def _target(spec): def test_decimal_ring_scores(self): """ - Check expected score can be calculated for non integer ring scores + Check expected score can be calculated for non integer ring scores. Uses a target with integer ring scores at twice the value for comparison """ @@ -421,9 +417,7 @@ def test_decimal_ring_scores(self): assert s_bar_int == 2 * s_bar_dec def test_array_handicaps(self): - """ - Check expected score can be calculated for an array of input handicap values - """ + """Check expected score can be calculated for an array of handicap values.""" handicaps = np.array([10, 20, 30, 40]) target = Target.from_spec({0.1: 3, 0.2: 5}, 10, 10) s_bar = hc.arrow_score(handicaps, target, "AGB") @@ -652,10 +646,7 @@ def test_rounded_round_score( ) def test_calculation_custom_scoring(self): - """ - Check that score can be calculated for a round with custom scoring - """ - + """Check that score can be calculated for a round with custom scoring.""" assert hc.score_for_round(20.0, kings_900_rec, "AGB", None, True) == 896.0 @@ -913,8 +904,5 @@ def test_decimal( assert handicap == pytest.approx(handicap_expected) def test_calculation_custom_scoring(self): - """ - Check that handicap can be calculated for a round with custom scoring - """ - + """Check that handicap can be calculated for a round with custom scoring.""" assert hc.handicap_from_score(896, kings_900_rec, "AGB", int_prec=True) == 20 diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 188bdf0..a2bad05 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -126,7 +126,6 @@ def __init__( distance: Union[float, tuple[float, str]], indoor: bool = False, ) -> None: - if scoring_system not in self.supported_systems: msg = ( f"""Invalid Target Face Type specified.\n""" @@ -204,7 +203,7 @@ def from_spec( Examples -------- - >>> #WA 18m compound triple spot + >>> # WA 18m compound triple spot >>> specs = {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} >>> target = Target.from_spec(specs, 40, 18) """ diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 5582c98..444709e 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -121,9 +121,7 @@ def test_properties(self) -> None: assert test_pass.native_diameter_unit == "cm" def test_custom_target(self) -> None: - """ - Check that pass can be constructed from a custom target specification - """ + """Check that pass can be constructed from a custom target specification.""" target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) test_pass = Pass(30, target) assert test_pass.target.is_custom diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 20ed7a0..a1f20dd 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -1,5 +1,7 @@ """Tests for Target class.""" +from typing import Final + import pytest from archeryutils.targets import ScoringSystem, Target @@ -251,9 +253,7 @@ def test_min_score_invalid_face_type(self) -> None: ], ) def test_get_face_spec(self, scoring_system, diam) -> None: - """ - Check that target returns correct face specifications from supported scoring systems. - """ + """Check that target returns face specs from supported scoring systems.""" expected_spec = { "5_zone": {0.244: 9, 0.488: 7, 0.732: 5, 0.976: 3, 1.22: 1}, "10_zone": { @@ -281,17 +281,12 @@ def test_get_face_spec(self, scoring_system, diam) -> None: class TestCustomScoringTarget: - """ - Tests for Target class with custom scoring - """ + """Tests for Target class with custom scoring.""" - _11zone_spec = {0.02: 11, 0.04: 10, 0.8: 9, 0.12: 8, 0.16: 7, 0.2: 6} + _11zone_spec: Final = {0.02: 11, 0.04: 10, 0.8: 9, 0.12: 8, 0.16: 7, 0.2: 6} def test_constructor(self) -> None: - """ - Can initialise Target with a custom scoring system and spec - """ - + """Can initialise Target with a custom scoring system and spec.""" target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) assert target.distance == 50.0 * 0.9144 assert target.diameter == 0.8 @@ -299,16 +294,12 @@ def test_constructor(self) -> None: assert target.get_face_spec() == {0.1: 3, 0.5: 1} def test_face_spec_units(self) -> None: - """ - Check custom Target can be constructed with alternative units. - """ + """Check custom Target can be constructed with alternative units.""" target = Target.from_spec(({10: 5, 20: 4, 30: 3}, "cm"), 50, 30) assert target.get_face_spec() == {0.1: 5, 0.2: 4, 0.3: 3} def test_invalid_face_spec_units(self) -> None: - """ - Check custom Target cannot be constructed with unsupported units. - """ + """Check custom Target cannot be constructed with unsupported units.""" with pytest.raises( ValueError, # match= @@ -333,23 +324,17 @@ def test_invalid_face_spec_units(self) -> None: ], ) def test_equality(self, spec, args, result) -> None: - """ - Check custom Target equality comparison is supported. - """ + """Check custom Target equality comparison is supported.""" target = Target.from_spec({0.2: 2, 0.4: 1}, 40, 20, indoor=True) comparison = target == Target.from_spec(spec, *args) assert comparison == result def test_max_score(self) -> None: - """ - Check that Target with custom scoring system returns correct max score - """ + """Check that Target with custom scoring system returns correct max score.""" target = Target.from_spec(self._11zone_spec, 40, 18) assert target.max_score() == 11 def test_min_score(self) -> None: - """ - Check that Target with custom scoring system returns correct min score - """ + """Check that Target with custom scoring system returns correct min score.""" target = Target.from_spec(self._11zone_spec, 40, 18) assert target.min_score() == 6 From 753639e15d42020900346cff8d6d07f02db32d87 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 21:34:13 +0000 Subject: [PATCH 06/43] Infer units in error messages for unsupported distance/diameter units --- archeryutils/constants.py | 22 ++++++++++++++++++++++ archeryutils/targets.py | 4 ++-- archeryutils/tests/test_targets.py | 7 +++---- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/archeryutils/constants.py b/archeryutils/constants.py index c3af139..288dcb0 100644 --- a/archeryutils/constants.py +++ b/archeryutils/constants.py @@ -153,3 +153,25 @@ def definitive_unit(cls, alias: str) -> str: "metre" """ return cls._reversed[alias] + + @classmethod + def definitive_units(cls, aliases: set[str]) -> set[str]: + """ + Reduce a set of string unit aliases to just their definitive names. + + Parameters + ---------- + aliases : set of str + names of units to be converted + + Returns + ------- + set of str + definitive names of unit + + Examples + -------- + >>> Length.definitive_unit(Length.metre | Length.yard) + {'metre', 'yard'} + """ + return {cls._reversed[alias] for alias in aliases} diff --git a/archeryutils/targets.py b/archeryutils/targets.py index a2bad05..2ae8906 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -141,7 +141,7 @@ def __init__( if native_dist_unit not in self.supported_distance_units: msg = ( f"Distance unit '{native_dist_unit}' not recognised. " - "Select from 'yard' or 'metre'." + f"Select from {Length.definitive_units(self.supported_distance_units)}." ) raise ValueError(msg) distance = Length.to_metres(distance, native_dist_unit) @@ -153,7 +153,7 @@ def __init__( if native_diameter_unit not in self.supported_diameter_units: msg = ( f"Diameter unit '{native_diameter_unit}' not recognised. " - "Select from 'cm', 'inch' or 'metre'" + f"Select from {Length.definitive_units(self.supported_diameter_units)}" ) raise ValueError(msg) diameter = Length.to_metres(diameter, native_diameter_unit) diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index a1f20dd..6317c95 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -101,7 +101,7 @@ def test_invalid_distance_unit(self) -> None: """Check that Target() returns error value for invalid distance units.""" with pytest.raises( ValueError, - match="Distance unit '(.+)' not recognised. Select from 'yard' or 'metre'.", + match="Distance unit '(.+)' not recognised. Select from", ): Target("5_zone", 122, (50, "InvalidDistanceUnit"), False) @@ -119,10 +119,9 @@ def test_unsupported_diameter_unit(self) -> None: """Check Target() raises error when called with unsupported diameter units.""" with pytest.raises( ValueError, - match="Diameter unit '(.+)' not recognised." - " Select from 'cm', 'inch' or 'metre'", + match="Diameter unit '(.+)' not recognised. Select from" ): - Target("5_zone", (122, "feet"), (50, "yards")) + Target("5_zone", (122, "bananas"), (50, "yards")) def test_default_diameter_unit(self) -> None: """Check that Target() is using centimetres by default for diameter.""" From 4d266d0c429f7731de99ca70d7c95ece25ea3e71 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 22:17:22 +0000 Subject: [PATCH 07/43] Clear up scoring system data into target class and test overlap with scoring systems --- archeryutils/targets.py | 58 +++++++++++++++--------------- archeryutils/tests/test_targets.py | 15 ++++++-- 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 2ae8906..33ce832 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -1,7 +1,8 @@ """Module for representing a Target for archery applications.""" from functools import partial -from typing import Literal, NamedTuple, Union, get_args +from types import MappingProxyType +from typing import Final, Literal, NamedTuple, Union, get_args from archeryutils.constants import Length @@ -25,29 +26,6 @@ # TypeAlias (annotate explicitly in py3.10+) FaceSpec = dict[float, int] - -class Rings(NamedTuple): - """Container for data on max and min scores on target face types.""" - - high: int - low: int - - -_scoring_system_data = { - "5_zone": Rings(high=9, low=1), - "10_zone": Rings(high=10, low=1), - "10_zone_compound": Rings(high=10, low=1), - "10_zone_6_ring": Rings(high=10, low=5), - "10_zone_5_ring": Rings(high=10, low=6), - "10_zone_5_ring_compound": Rings(high=10, low=6), - "WA_field": Rings(high=6, low=1), - "IFAA_field": Rings(high=5, low=3), - "IFAA_field_expert": Rings(high=5, low=1), - "Beiter_hit_miss": Rings(high=1, low=1), - "Worcester": Rings(high=5, low=1), - "Worcester_2_ring": Rings(high=5, low=4), -} - _rnd6 = partial(round, ndigits=6) @@ -115,6 +93,26 @@ class Target: """ _face_spec: FaceSpec + + # Lookup data for min and max scores for supported scoring systems. + # Used to deduplicate logic from max_score, min_score and get_face_spec methods + _scoring_system_data: Final = MappingProxyType( + { + "5_zone": {"high": 9, "low": 1}, + "10_zone": {"high": 10, "low": 1}, + "10_zone_compound": {"high": 10, "low": 1}, + "10_zone_6_ring": {"high": 10, "low": 5}, + "10_zone_5_ring": {"high": 10, "low": 6}, + "10_zone_5_ring_compound": {"high": 10, "low": 6}, + "WA_field": {"high": 6, "low": 1}, + "IFAA_field": {"high": 5, "low": 3}, + "IFAA_field_expert": {"high": 5, "low": 1}, + "Beiter_hit_miss": {"high": 1, "low": 1}, + "Worcester": {"high": 5, "low": 1}, + "Worcester_2_ring": {"high": 5, "low": 4}, + } + ) + supported_systems = get_args(ScoringSystem) supported_distance_units = Length.yard | Length.metre supported_diameter_units = Length.cm | Length.inch | Length.metre @@ -303,9 +301,9 @@ def max_score(self) -> float: """ if self.is_custom: return max(self._face_spec.values()) - data = _scoring_system_data.get(self.scoring_system) + data = self._scoring_system_data.get(self.scoring_system) if data: - return data.high + return data["high"] # NB: Should be hard (but not impossible) to get here without catching earlier. msg = f"Target face '{self.scoring_system}' has no specified maximum score." raise ValueError(msg) @@ -332,9 +330,9 @@ def min_score(self) -> float: """ if self.is_custom: return min(self._face_spec.values()) - data = _scoring_system_data.get(self.scoring_system) + data = self._scoring_system_data.get(self.scoring_system) if data: - return data.low + return data["low"] # NB: Should be hard (but not impossible) to get here without catching earlier. msg = f"Target face '{self.scoring_system}' has no specified minimum score." raise ValueError(msg) @@ -354,14 +352,14 @@ def get_face_spec(self) -> FaceSpec: if system == "Custom": return self._face_spec - data = _scoring_system_data.get(system) + data = self._scoring_system_data.get(system) if not data: # no data for scoring system, raise msg = f"No rule for calculating scoring for face type {system}." raise ValueError(msg) # calculate missing rings for certain targets from minimum score - missing = data.low - 1 + missing = data["low"] - 1 if system == "5_zone": spec = {_rnd6((n + 1) * tar_dia / 10): 10 - n for n in range(1, 11, 2)} diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 6317c95..8930afd 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -1,6 +1,6 @@ """Tests for Target class.""" -from typing import Final +from typing import Final, get_args import pytest @@ -118,8 +118,7 @@ def test_yard_to_m_conversion(self) -> None: def test_unsupported_diameter_unit(self) -> None: """Check Target() raises error when called with unsupported diameter units.""" with pytest.raises( - ValueError, - match="Diameter unit '(.+)' not recognised. Select from" + ValueError, match="Diameter unit '(.+)' not recognised. Select from" ): Target("5_zone", (122, "bananas"), (50, "yards")) @@ -337,3 +336,13 @@ def test_min_score(self) -> None: """Check that Target with custom scoring system returns correct min score.""" target = Target.from_spec(self._11zone_spec, 40, 18) assert target.min_score() == 6 + + +class TestTargetData: + """Class to test the provided scoring system data.""" + + def test_all_systems_present(self) -> None: + """Check that all listed scoring systems have data available.""" + data = Target._scoring_system_data # noqa: SLF001 + for system in Target.supported_systems: + assert system in data or system == "Custom" From 79dd1aa45cc2b899e01b2da75208eb91c00a54f3 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 22:21:21 +0000 Subject: [PATCH 08/43] Formatting, Rename alternative target constructor --- archeryutils/handicaps/handicap_scheme.py | 6 +- .../handicaps/tests/test_handicaps.py | 20 +-- archeryutils/targets.py | 4 +- archeryutils/tests/test_rounds.py | 4 +- archeryutils/tests/test_targets.py | 143 ++++++++++++------ 5 files changed, 112 insertions(+), 65 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index b0f1229..990448f 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -107,8 +107,7 @@ def __repr__(self) -> str: @overload @abstractmethod - def sigma_t(self, handicap: float, dist: float) -> float: - ... + def sigma_t(self, handicap: float, dist: float) -> float: ... @overload @abstractmethod @@ -116,8 +115,7 @@ def sigma_t( self, handicap: npt.NDArray[np.float64], dist: float, - ) -> npt.NDArray[np.float64]: - ... + ) -> npt.NDArray[np.float64]: ... @abstractmethod def sigma_t(self, handicap: FloatArray, dist: float) -> FloatArray: diff --git a/archeryutils/handicaps/tests/test_handicaps.py b/archeryutils/handicaps/tests/test_handicaps.py index 98f5110..501e86a 100644 --- a/archeryutils/handicaps/tests/test_handicaps.py +++ b/archeryutils/handicaps/tests/test_handicaps.py @@ -76,10 +76,12 @@ kings_900_rec = Round( "Kings 900 (recurve)", [ - Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), - Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), - Pass(30, Target.from_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True)), - ], + Pass( + 30, + Target.from_face_spec({0.08: 10, 0.12: 8, 0.16: 7, 0.20: 6}, 40, 18, True), + ) + ] + * 3, ) @@ -386,7 +388,7 @@ def test_different_target_faces( def test_empty_spec(self): """Check expected score is zero when no target rings are defined.""" - target = Target.from_spec({}, 10, 10) + target = Target.from_face_spec({}, 10, 10) s_bar = hc.arrow_score(50, target, "AGB") assert s_bar == 0 @@ -394,7 +396,7 @@ def test_unsorted_spec(self): """Check expected score is insensitive to order of input spec.""" def _target(spec): - return Target.from_spec(spec, 10, 10) + return Target.from_face_spec(spec, 10, 10) s_bar = hc.arrow_score(50, _target({0.1: 1, 0.2: 2, 0.3: 3}), "AA") s_bar_reversed = hc.arrow_score(50, _target({0.3: 3, 0.2: 2, 0.1: 1}), "AA") @@ -408,8 +410,8 @@ def test_decimal_ring_scores(self): Uses a target with integer ring scores at twice the value for comparison """ - target_int = Target.from_spec({0.1: 3, 0.2: 5}, 10, 10) - target_dec = Target.from_spec({0.1: 1.5, 0.2: 2.5}, 10, 10) + target_int = Target.from_face_spec({0.1: 3, 0.2: 5}, 10, 10) + target_dec = Target.from_face_spec({0.1: 1.5, 0.2: 2.5}, 10, 10) s_bar_int = hc.arrow_score(30, target_int, "AGB") s_bar_dec = hc.arrow_score(30, target_dec, "AGB") @@ -419,7 +421,7 @@ def test_decimal_ring_scores(self): def test_array_handicaps(self): """Check expected score can be calculated for an array of handicap values.""" handicaps = np.array([10, 20, 30, 40]) - target = Target.from_spec({0.1: 3, 0.2: 5}, 10, 10) + target = Target.from_face_spec({0.1: 3, 0.2: 5}, 10, 10) s_bar = hc.arrow_score(handicaps, target, "AGB") assert len(s_bar) == len(handicaps) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 33ce832..6791823 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -164,7 +164,7 @@ def __init__( self.indoor = indoor @classmethod - def from_spec( + def from_face_spec( cls, face_spec: Union[FaceSpec, tuple[FaceSpec, str]], diameter: Union[float, tuple[float, str]], @@ -182,7 +182,7 @@ def from_spec( Parameters ---------- - face_spec : dict of floats to ints or 2-tuple of dict, str + face_spec : FaceSpec or 2-tuple of FaceSpec, str Target face specification, a mapping of target ring sizes to score. Default units are assumed as [metres] but can be provided as the second element of a tuple. diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 444709e..92c1bd8 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -22,8 +22,6 @@ class TestPass: test behaviour of default location test_negative_arrows() test behaviour of negative arrow number - test_custom_constructor() - test construction from a custom target specification test_properties() test setting of Pass properties test_max_score() @@ -122,7 +120,7 @@ def test_properties(self) -> None: def test_custom_target(self) -> None: """Check that pass can be constructed from a custom target specification.""" - target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) + target = Target.from_face_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) test_pass = Pass(30, target) assert test_pass.target.is_custom diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 8930afd..8b8c379 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -100,8 +100,7 @@ def test_invalid_system(self) -> None: def test_invalid_distance_unit(self) -> None: """Check that Target() returns error value for invalid distance units.""" with pytest.raises( - ValueError, - match="Distance unit '(.+)' not recognised. Select from", + ValueError, match="Distance unit '(.+)' not recognised. Select from" ): Target("5_zone", 122, (50, "InvalidDistanceUnit"), False) @@ -238,44 +237,97 @@ def test_min_score_invalid_face_type(self) -> None: target.min_score() @pytest.mark.parametrize( - "scoring_system, diam", + "scoring_system, diam, expected_spec", [ - ("5_zone", 122), - ("10_zone", 80), - ("WA_field", 80), - ("IFAA_field", 50), - ("Beiter_hit_miss", 6), - ("Worcester", (16, "inch")), - ("10_zone_6_ring", 80), - ("10_zone_5_ring_compound", 40), + ( + "5_zone", + 122, + { + 0.244: 9, + 0.488: 7, + 0.732: 5, + 0.976: 3, + 1.22: 1, + }, + ), + ( + "10_zone", + 80, + { + 0.08: 10, + 0.16: 9, + 0.24: 8, + 0.32: 7, + 0.4: 6, + 0.48: 5, + 0.56: 4, + 0.64: 3, + 0.72: 2, + 0.8: 1, + }, + ), + ( + "WA_field", + 80, + { + 0.08: 6, + 0.16: 5, + 0.32: 4, + 0.48: 3, + 0.64: 2, + 0.8: 1, + }, + ), + ( + "IFAA_field", + 50, + { + 0.1: 5, + 0.3: 4, + 0.5: 3, + }, + ), + ( + "Beiter_hit_miss", + 6, + { + 0.06: 1, + }, + ), + ( + "Worcester", + (16, "inch"), + { + 0.08128: 5, + 0.16256: 4, + 0.24384: 3, + 0.32512: 2, + 0.4064: 1, + }, + ), + ( + "10_zone_6_ring", + 80, + { + 0.08: 10, + 0.16: 9, + 0.24: 8, + 0.32: 7, + 0.4: 6, + 0.48: 5, + }, + ), + ( + "10_zone_5_ring_compound", + 40, + {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6}, + ), ], ) - def test_get_face_spec(self, scoring_system, diam) -> None: + def test_get_face_spec(self, scoring_system, diam, expected_spec) -> None: """Check that target returns face specs from supported scoring systems.""" - expected_spec = { - "5_zone": {0.244: 9, 0.488: 7, 0.732: 5, 0.976: 3, 1.22: 1}, - "10_zone": { - 0.08: 10, - 0.16: 9, - 0.24: 8, - 0.32: 7, - 0.4: 6, - 0.48: 5, - 0.56: 4, - 0.64: 3, - 0.72: 2, - 0.8: 1, - }, - "WA_field": {0.08: 6, 0.16: 5, 0.32: 4, 0.48: 3, 0.64: 2, 0.8: 1}, - "IFAA_field": {0.1: 5, 0.3: 4, 0.5: 3}, - "Beiter_hit_miss": {0.06: 1}, - "Worcester": {0.08128: 5, 0.16256: 4, 0.24384: 3, 0.32512: 2, 0.4064: 1}, - "10_zone_6_ring": {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6, 0.48: 5}, - "10_zone_5_ring_compound": {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6}, - } - target = Target(scoring_system, diam, 30) - assert target.get_face_spec() == expected_spec[scoring_system] + assert target.get_face_spec() == expected_spec class TestCustomScoringTarget: @@ -284,8 +336,8 @@ class TestCustomScoringTarget: _11zone_spec: Final = {0.02: 11, 0.04: 10, 0.8: 9, 0.12: 8, 0.16: 7, 0.2: 6} def test_constructor(self) -> None: - """Can initialise Target with a custom scoring system and spec.""" - target = Target.from_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) + """Check initialisation of Target with a custom scoring system and spec.""" + target = Target.from_face_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) assert target.distance == 50.0 * 0.9144 assert target.diameter == 0.8 assert target.scoring_system == "Custom" @@ -293,16 +345,13 @@ def test_constructor(self) -> None: def test_face_spec_units(self) -> None: """Check custom Target can be constructed with alternative units.""" - target = Target.from_spec(({10: 5, 20: 4, 30: 3}, "cm"), 50, 30) + target = Target.from_face_spec(({10: 5, 20: 4, 30: 3}, "cm"), 50, 30) assert target.get_face_spec() == {0.1: 5, 0.2: 4, 0.3: 3} def test_invalid_face_spec_units(self) -> None: """Check custom Target cannot be constructed with unsupported units.""" - with pytest.raises( - ValueError, - # match= - ): - Target.from_spec(({10: 5, 20: 4, 30: 3}, "bananas"), 50, 30) + with pytest.raises(ValueError): + Target.from_face_spec(({10: 5, 20: 4, 30: 3}, "bananas"), 50, 30) @pytest.mark.parametrize( "spec, args, result", @@ -323,18 +372,18 @@ def test_invalid_face_spec_units(self) -> None: ) def test_equality(self, spec, args, result) -> None: """Check custom Target equality comparison is supported.""" - target = Target.from_spec({0.2: 2, 0.4: 1}, 40, 20, indoor=True) - comparison = target == Target.from_spec(spec, *args) + target = Target.from_face_spec({0.2: 2, 0.4: 1}, 40, 20, indoor=True) + comparison = target == Target.from_face_spec(spec, *args) assert comparison == result def test_max_score(self) -> None: """Check that Target with custom scoring system returns correct max score.""" - target = Target.from_spec(self._11zone_spec, 40, 18) + target = Target.from_face_spec(self._11zone_spec, 40, 18) assert target.max_score() == 11 def test_min_score(self) -> None: """Check that Target with custom scoring system returns correct min score.""" - target = Target.from_spec(self._11zone_spec, 40, 18) + target = Target.from_face_spec(self._11zone_spec, 40, 18) assert target.min_score() == 6 From ffe72bb152b1ab29ff8d1c805f93724ce68a76ba Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 22:51:56 +0000 Subject: [PATCH 09/43] Explicit test for get_face_spec on an invalid scoring system --- archeryutils/tests/test_targets.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 8b8c379..ffcaf99 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -329,6 +329,14 @@ def test_get_face_spec(self, scoring_system, diam, expected_spec) -> None: target = Target(scoring_system, diam, 30) assert target.get_face_spec() == expected_spec + def test_get_face_spec_invalid_system(self) -> None: + """Check error is raised when trying to get specs of an unsupported system.""" + target = Target("5_zone", 122, 50) + # Silence mypy as scoring_system must be a valid literal ScoringSystem + target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] + with pytest.raises(ValueError): + target.get_face_spec() + class TestCustomScoringTarget: """Tests for Target class with custom scoring.""" From 13fbad66d1b149bef4f26da8ddf6e9a3f89cbb1a Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 22:55:48 +0000 Subject: [PATCH 10/43] Simplify return statement in arrow_score --- archeryutils/handicaps/handicap_scheme.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index 990448f..f455b76 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -231,8 +231,7 @@ def arrow_score( arw_rad = arw_d / 2.0 spec = target.get_face_spec() sig_r = self.sigma_r(handicap, target.distance) - s_bar = self._s_bar(spec, arw_rad, sig_r) - return s_bar + return self._s_bar(spec, arw_rad, sig_r) def _s_bar( self, target_specs: dict[float, int], arw_rad: float, sig_r: FloatArray From 86214dece813670ce5a6fdfc755c8aab5443b0f6 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 22:56:51 +0000 Subject: [PATCH 11/43] Reactivate commented equality test in Pass.at_target constructor test --- archeryutils/tests/test_rounds.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 92c1bd8..882566a 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -40,8 +40,7 @@ def test_at_target_constructor(self) -> None: test_pass = Pass.at_target(36, "5_zone", 122, 50) assert test_pass.n_arrows == 36 - # cannot test for equality between targets as __eq__ not implemented - # assert test_pass.target == _target + assert test_pass.target == _target def test_repr(self) -> None: """Check Pass string representation.""" From 9e4e5e4dc7ffb08d6743094771a17e948fdf9d0c Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 6 Mar 2024 23:01:57 +0000 Subject: [PATCH 12/43] fixup: also derive units in error message on from_face_spec constructor --- archeryutils/targets.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 6791823..261d79d 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -211,7 +211,8 @@ def from_face_spec( if spec_units not in cls.supported_diameter_units: msg = ( f"Face specification unit '{spec_units}' not recognised. " - "Select from 'cm', 'inch' or 'metre'" + "Select from " + f"{Length.definitive_units(cls.supported_diameter_units)}" ) raise ValueError(msg) face_spec = { From 5f2575a36479d2d166f1fcff28c767ed179f6b8c Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Thu, 7 Mar 2024 21:30:01 +0000 Subject: [PATCH 13/43] Refactor optional tuple parsing logic from Target to Length class --- archeryutils/constants.py | 63 +++++++++++++++++++++++++++++- archeryutils/targets.py | 62 +++++++++-------------------- archeryutils/tests/test_targets.py | 10 ++--- 3 files changed, 83 insertions(+), 52 deletions(-) diff --git a/archeryutils/constants.py b/archeryutils/constants.py index 288dcb0..c2aac6d 100644 --- a/archeryutils/constants.py +++ b/archeryutils/constants.py @@ -1,6 +1,8 @@ """Constants used in the archeryutils package.""" -from typing import ClassVar +from typing import Any, ClassVar, TypeVar, Union + +T = TypeVar("T") _CONVERSIONS_TO_M = { "metre": 1.0, @@ -175,3 +177,62 @@ def definitive_units(cls, aliases: set[str]) -> set[str]: {'metre', 'yard'} """ return {cls._reversed[alias] for alias in aliases} + + @classmethod + def parse_optional_units( + cls, + value: Union[T, tuple[T, str]], + supported: set[str], + default: str, + ) -> tuple[T, str]: + """ + Parse single value or tuple of value and units. + + Always returns a tuple of value and units + + Parameters + ---------- + value : Any or tuple of Any, str + Either a single object, or a tuple with the desired units + supported: set of str + Valid unit aliases to be accepted + default: str + Default unit to be used when value does not specify units. + + Raises + ------ + ValueError + If default units or parsed values units + are not contained in supported units. + + Returns + ------- + tuple of Any, str + original value, definitive name of unit + + Examples + -------- + >>> m_and_yd = Length.metre | Length.yard + >>> Length.parse_optional_units(10, m_and_yd, "metre") + (10, 'metre') + >>> Length.parse_optional_units((10, 'yards') m_and_yd, 'metre') + (10, 'yard') + >>> Length.parse_optional_units((10, 'banana') m_and_yd, 'metre') + ValueError: Unit 'banana' not recognised. Select from {'yard', 'metre'}. + """ + if default not in supported: + msg = f"Default unit {default!r} must be in supported units" + raise ValueError(msg) + if isinstance(value, tuple) and len(value) == 2: # noqa: PLR2004 + quantity, units = value + else: + quantity = value + units = default + + if units not in supported: + msg = ( + f"Unit {units!r} not recognised. " + f"Select from {cls.definitive_units(supported)}." + ) + raise ValueError(msg) + return quantity, cls.definitive_unit(units) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 261d79d..f3376d9 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -130,37 +130,18 @@ def __init__( f"""Please select from '{"', '".join(self.supported_systems)}'.""" ) raise ValueError(msg) - - if isinstance(distance, tuple): - (distance, native_dist_unit) = distance - else: - native_dist_unit = "metre" - - if native_dist_unit not in self.supported_distance_units: - msg = ( - f"Distance unit '{native_dist_unit}' not recognised. " - f"Select from {Length.definitive_units(self.supported_distance_units)}." - ) - raise ValueError(msg) - distance = Length.to_metres(distance, native_dist_unit) - - if isinstance(diameter, tuple): - (diameter, native_diameter_unit) = diameter - else: - native_diameter_unit = "cm" - if native_diameter_unit not in self.supported_diameter_units: - msg = ( - f"Diameter unit '{native_diameter_unit}' not recognised. " - f"Select from {Length.definitive_units(self.supported_diameter_units)}" - ) - raise ValueError(msg) - diameter = Length.to_metres(diameter, native_diameter_unit) + dist, native_dist_unit = Length.parse_optional_units( + distance, self.supported_distance_units, "metre" + ) + diam, native_diameter_unit = Length.parse_optional_units( + diameter, self.supported_diameter_units, "cm" + ) self.scoring_system = scoring_system - self.diameter = diameter - self.native_diameter_unit = Length.definitive_unit(native_diameter_unit) - self.distance = distance - self.native_dist_unit = Length.definitive_unit(native_dist_unit) + self.distance = Length.to_metres(dist, native_dist_unit) + self.native_dist_unit = native_dist_unit + self.diameter = Length.to_metres(diam, native_diameter_unit) + self.native_diameter_unit = native_diameter_unit self.indoor = indoor @classmethod @@ -205,23 +186,16 @@ def from_face_spec( >>> specs = {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} >>> target = Target.from_spec(specs, 40, 18) """ - if isinstance(face_spec, tuple): - spec_data, spec_units = face_spec - - if spec_units not in cls.supported_diameter_units: - msg = ( - f"Face specification unit '{spec_units}' not recognised. " - "Select from " - f"{Length.definitive_units(cls.supported_diameter_units)}" - ) - raise ValueError(msg) - face_spec = { - _rnd6(Length.to_metres(ring_diam, spec_units)): score - for ring_diam, score in spec_data.items() - } + spec_data, spec_units = Length.parse_optional_units( + face_spec, cls.supported_diameter_units, "metre" + ) + spec = { + _rnd6(Length.to_metres(ring_diam, spec_units)): score + for ring_diam, score in spec_data.items() + } target = cls("Custom", diameter, distance, indoor) - target._face_spec = face_spec # noqa: SLF001 private member access + target._face_spec = spec # noqa: SLF001 private member access return target def __repr__(self) -> str: diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index ffcaf99..e1688d7 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -99,9 +99,7 @@ def test_invalid_system(self) -> None: def test_invalid_distance_unit(self) -> None: """Check that Target() returns error value for invalid distance units.""" - with pytest.raises( - ValueError, match="Distance unit '(.+)' not recognised. Select from" - ): + with pytest.raises(ValueError, match="Unit '(.+)' not recognised. Select from"): Target("5_zone", 122, (50, "InvalidDistanceUnit"), False) def test_default_distance_unit(self) -> None: @@ -114,11 +112,9 @@ def test_yard_to_m_conversion(self) -> None: target = Target("5_zone", 122, (50, "yards")) assert target.distance == 50.0 * 0.9144 - def test_unsupported_diameter_unit(self) -> None: + def test_invalid_diameter_unit(self) -> None: """Check Target() raises error when called with unsupported diameter units.""" - with pytest.raises( - ValueError, match="Diameter unit '(.+)' not recognised. Select from" - ): + with pytest.raises(ValueError, match="Unit '(.+)' not recognised. Select from"): Target("5_zone", (122, "bananas"), (50, "yards")) def test_default_diameter_unit(self) -> None: From 0086d160feecdb65f0505a2927d730d3a8500783 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Thu, 7 Mar 2024 23:42:18 +0000 Subject: [PATCH 14/43] Refactor face_spec to property and set in all instances Able to remove _scoring_system_data dict and just inline the few required values into face spec generator. face_spec property now generated lazily on demand. --- archeryutils/handicaps/handicap_scheme.py | 2 +- archeryutils/targets.py | 129 ++++++++++------------ archeryutils/tests/test_targets.py | 30 ++--- 3 files changed, 68 insertions(+), 93 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index f455b76..4135bd0 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -229,7 +229,7 @@ def arrow_score( arw_d = self.arw_d_out arw_rad = arw_d / 2.0 - spec = target.get_face_spec() + spec = target.face_spec sig_r = self.sigma_r(handicap, target.distance) return self._s_bar(spec, arw_rad, sig_r) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index f3376d9..251a275 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -94,25 +94,6 @@ class Target: _face_spec: FaceSpec - # Lookup data for min and max scores for supported scoring systems. - # Used to deduplicate logic from max_score, min_score and get_face_spec methods - _scoring_system_data: Final = MappingProxyType( - { - "5_zone": {"high": 9, "low": 1}, - "10_zone": {"high": 10, "low": 1}, - "10_zone_compound": {"high": 10, "low": 1}, - "10_zone_6_ring": {"high": 10, "low": 5}, - "10_zone_5_ring": {"high": 10, "low": 6}, - "10_zone_5_ring_compound": {"high": 10, "low": 6}, - "WA_field": {"high": 6, "low": 1}, - "IFAA_field": {"high": 5, "low": 3}, - "IFAA_field_expert": {"high": 5, "low": 1}, - "Beiter_hit_miss": {"high": 1, "low": 1}, - "Worcester": {"high": 5, "low": 1}, - "Worcester_2_ring": {"high": 5, "low": 4}, - } - ) - supported_systems = get_args(ScoringSystem) supported_distance_units = Length.yard | Length.metre supported_diameter_units = Length.cm | Length.inch | Length.metre @@ -213,14 +194,15 @@ def __repr__(self) -> str: def __eq__(self, other: object) -> bool: """Check equality of Targets based on parameters.""" - if isinstance(other, Target): - if self.scoring_system == other.scoring_system == "Custom": - return ( - self._face_spec == other._face_spec - and self._parameters() == other._parameters() - ) - return self._parameters() == other._parameters() - return NotImplemented + if not isinstance(other, Target): + return NotImplemented + + if self.scoring_system == other.scoring_system == "Custom": + return ( + self._face_spec == other._face_spec + and self._parameters() == other._parameters() + ) + return self._parameters() == other._parameters() def _parameters(self): """Shortcut to get all target parameters as a tuple for comparison.""" @@ -274,14 +256,7 @@ def max_score(self) -> float: >>> mytarget.max_score() 10.0 """ - if self.is_custom: - return max(self._face_spec.values()) - data = self._scoring_system_data.get(self.scoring_system) - if data: - return data["high"] - # NB: Should be hard (but not impossible) to get here without catching earlier. - msg = f"Target face '{self.scoring_system}' has no specified maximum score." - raise ValueError(msg) + return max(self.face_spec.values()) def min_score(self) -> float: """ @@ -303,62 +278,78 @@ def min_score(self) -> float: >>> mytarget.min_score() 1.0 """ + return min(self.face_spec.values()) + + @property + def face_spec(self) -> FaceSpec: + """Get the targets face specification.""" + if not hasattr(self, "_face_spec"): + self._face_spec = self._get_face_spec() + return self._face_spec + + def _get_face_spec(self): + """Generate face specification on demand from instance parameters.""" if self.is_custom: - return min(self._face_spec.values()) - data = self._scoring_system_data.get(self.scoring_system) - if data: - return data["low"] - # NB: Should be hard (but not impossible) to get here without catching earlier. - msg = f"Target face '{self.scoring_system}' has no specified minimum score." - raise ValueError(msg) - - def get_face_spec(self) -> FaceSpec: - # Could replace with mapping face to lambda that returns spec - """Derive specifications for common/supported targets. + return self.face_spec + + return self.gen_face_spec(self.scoring_system, self.diameter) + + @staticmethod + def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: + """ + Derive specifications for common/supported targets. + + Parameters + ---------- + system: ScoringSystem + Name of scoring system + diameter: + Target diameter in metres Returns ------- spec : dict Mapping of target ring sizes in [metres] to score """ - system = self.scoring_system - tar_dia = self.diameter - - if system == "Custom": - return self._face_spec - - data = self._scoring_system_data.get(system) - if not data: - # no data for scoring system, raise - msg = f"No rule for calculating scoring for face type {system}." - raise ValueError(msg) - - # calculate missing rings for certain targets from minimum score - missing = data["low"] - 1 + removed_rings = { + "10_zone_6_ring": 4, + "10_zone_5_ring": 5, + "10_zone_5_ring_compound": 5, + "Worcester_2_ring": 3, + } + missing = removed_rings.get(system, 0) if system == "5_zone": - spec = {_rnd6((n + 1) * tar_dia / 10): 10 - n for n in range(1, 11, 2)} + spec = {_rnd6((n + 1) * diameter / 10): 10 - n for n in range(1, 11, 2)} elif system in ("10_zone", "10_zone_6_ring", "10_zone_5_ring"): - spec = {_rnd6(n * tar_dia / 10): 11 - n for n in range(1, 11 - missing)} + spec = {_rnd6(n * diameter / 10): 11 - n for n in range(1, 11 - missing)} elif system in ("10_zone_compound", "10_zone_5_ring_compound"): - spec = {_rnd6(tar_dia / 20): 10} | { - _rnd6(n * tar_dia / 10): 11 - n for n in range(2, 11 - missing) + spec = {_rnd6(diameter / 20): 10} | { + _rnd6(n * diameter / 10): 11 - n for n in range(2, 11 - missing) } elif system == "WA_field": - spec = {_rnd6(tar_dia / 10): 6} | { - _rnd6(n * tar_dia / 5): 6 - n for n in range(1, 6) + spec = {_rnd6(diameter / 10): 6} | { + _rnd6(n * diameter / 5): 6 - n for n in range(1, 6) } elif system == "IFAA_field": - spec = {_rnd6(n * tar_dia / 5): 5 - n // 2 for n in range(1, 6, 2)} + spec = {_rnd6(n * diameter / 5): 5 - n // 2 for n in range(1, 6, 2)} elif system == "Beiter_hit_miss": - spec = {tar_dia: 1} + spec = {diameter: 1} elif system in ("Worcester", "Worcester_2_ring", "IFAA_field_expert"): - spec = {_rnd6(n * tar_dia / 5): 6 - n for n in range(1, 6 - missing)} + spec = {_rnd6(n * diameter / 5): 6 - n for n in range(1, 6 - missing)} + + # NB: Should be hard (but not impossible) to get here without catching earlier; + # Can only occur if target scoring system is modified after initialisation + # Or newly supported scoring system doesn't have an implementation + # here for generating specs + else: + msg = f"Scoring system {system!r} is not supported" + raise ValueError(msg) return spec diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index e1688d7..5372b40 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -184,10 +184,7 @@ def test_max_score( def test_max_score_invalid_face_type(self) -> None: """Check that Target() raises error for invalid face.""" - with pytest.raises( - ValueError, - match="Target face '(.+)' has no specified maximum score.", - ): + with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): target = Target("5_zone", 122, 50, False) # Requires manual resetting of scoring system to get this error. # Silence mypy as scoring_system must be a valid literal ScoringSystem @@ -222,10 +219,7 @@ def test_min_score( def test_min_score_invalid_face_type(self) -> None: """Check that Target() raises error for invalid face.""" - with pytest.raises( - ValueError, - match="Target face '(.+)' has no specified minimum score.", - ): + with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): target = Target("5_zone", 122, 50, False) # Requires manual resetting of scoring system to get this error. # Silence mypy as scoring_system must be a valid literal ScoringSystem @@ -323,15 +317,15 @@ def test_min_score_invalid_face_type(self) -> None: def test_get_face_spec(self, scoring_system, diam, expected_spec) -> None: """Check that target returns face specs from supported scoring systems.""" target = Target(scoring_system, diam, 30) - assert target.get_face_spec() == expected_spec + assert target.face_spec == expected_spec def test_get_face_spec_invalid_system(self) -> None: """Check error is raised when trying to get specs of an unsupported system.""" target = Target("5_zone", 122, 50) # Silence mypy as scoring_system must be a valid literal ScoringSystem target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] - with pytest.raises(ValueError): - target.get_face_spec() + with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): + assert target.face_spec class TestCustomScoringTarget: @@ -345,12 +339,12 @@ def test_constructor(self) -> None: assert target.distance == 50.0 * 0.9144 assert target.diameter == 0.8 assert target.scoring_system == "Custom" - assert target.get_face_spec() == {0.1: 3, 0.5: 1} + assert target.face_spec == {0.1: 3, 0.5: 1} def test_face_spec_units(self) -> None: """Check custom Target can be constructed with alternative units.""" target = Target.from_face_spec(({10: 5, 20: 4, 30: 3}, "cm"), 50, 30) - assert target.get_face_spec() == {0.1: 5, 0.2: 4, 0.3: 3} + assert target.face_spec == {0.1: 5, 0.2: 4, 0.3: 3} def test_invalid_face_spec_units(self) -> None: """Check custom Target cannot be constructed with unsupported units.""" @@ -389,13 +383,3 @@ def test_min_score(self) -> None: """Check that Target with custom scoring system returns correct min score.""" target = Target.from_face_spec(self._11zone_spec, 40, 18) assert target.min_score() == 6 - - -class TestTargetData: - """Class to test the provided scoring system data.""" - - def test_all_systems_present(self) -> None: - """Check that all listed scoring systems have data available.""" - data = Target._scoring_system_data # noqa: SLF001 - for system in Target.supported_systems: - assert system in data or system == "Custom" From e63fa18bccbe9dfaebc309305cf7a3adb7b105c7 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Fri, 8 Mar 2024 07:51:52 +0000 Subject: [PATCH 15/43] Manage diameter in property to recaulate face spec after changes --- archeryutils/constants.py | 2 +- archeryutils/targets.py | 34 +++++++++++++++++------------- archeryutils/tests/test_targets.py | 9 +++++++- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/archeryutils/constants.py b/archeryutils/constants.py index c2aac6d..fc8f7f9 100644 --- a/archeryutils/constants.py +++ b/archeryutils/constants.py @@ -1,6 +1,6 @@ """Constants used in the archeryutils package.""" -from typing import Any, ClassVar, TypeVar, Union +from typing import ClassVar, TypeVar, Union T = TypeVar("T") diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 251a275..db98dba 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -1,8 +1,7 @@ """Module for representing a Target for archery applications.""" from functools import partial -from types import MappingProxyType -from typing import Final, Literal, NamedTuple, Union, get_args +from typing import Literal, Optional, Union, get_args from archeryutils.constants import Length @@ -92,7 +91,7 @@ class Target: """ - _face_spec: FaceSpec + _face_spec: Optional[FaceSpec] = None supported_systems = get_args(ScoringSystem) supported_distance_units = Length.yard | Length.metre @@ -121,7 +120,7 @@ def __init__( self.scoring_system = scoring_system self.distance = Length.to_metres(dist, native_dist_unit) self.native_dist_unit = native_dist_unit - self.diameter = Length.to_metres(diam, native_diameter_unit) + self._diameter = Length.to_metres(diam, native_diameter_unit) self.native_diameter_unit = native_diameter_unit self.indoor = indoor @@ -220,6 +219,18 @@ def is_custom(self): """Check if this Target uses a custom scoring system.""" return self.scoring_system == "Custom" + @property + def diameter(self): + """Get target diameter.""" + return self._diameter + + @diameter.setter + def diameter(self, value): + """Set target diameter and invalidate face spec for standard targets.""" + if self.scoring_system != "Custom": + self._face_spec = None + self._diameter = value + @property def native_distance(self) -> tuple[float, str]: """Get target distance in original native units.""" @@ -232,7 +243,7 @@ def native_distance(self) -> tuple[float, str]: def native_diameter(self) -> tuple[float, str]: """Get target diameter in original native units.""" return ( - Length.from_metres(self.diameter, self.native_diameter_unit), + Length.from_metres(self._diameter, self.native_diameter_unit), self.native_diameter_unit, ) @@ -282,18 +293,11 @@ def min_score(self) -> float: @property def face_spec(self) -> FaceSpec: - """Get the targets face specification.""" - if not hasattr(self, "_face_spec"): - self._face_spec = self._get_face_spec() + """Get the targets face specification, generating on demand if needed.""" + if self._face_spec is None and self.scoring_system != "Custom": + self._face_spec = self.gen_face_spec(self.scoring_system, self._diameter) return self._face_spec - def _get_face_spec(self): - """Generate face specification on demand from instance parameters.""" - if self.is_custom: - return self.face_spec - - return self.gen_face_spec(self.scoring_system, self.diameter) - @staticmethod def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: """ diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 5372b40..ab071e4 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -1,6 +1,6 @@ """Tests for Target class.""" -from typing import Final, get_args +from typing import Final import pytest @@ -327,6 +327,13 @@ def test_get_face_spec_invalid_system(self) -> None: with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): assert target.face_spec + def test_face_spec_recaluclated_after_changing_diameter(self) -> None: + """Check that face_spec respects the current target diameter.""" + target = Target("10_zone_5_ring", 80, 50) + assert target.face_spec == {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6} + target.diameter /= 2 + assert target.face_spec == {0.04: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} + class TestCustomScoringTarget: """Tests for Target class with custom scoring.""" From 1cb4052ca2942f9112224d9ffd0a9b2ee45221c8 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Fri, 8 Mar 2024 09:31:39 +0000 Subject: [PATCH 16/43] Manage distance and scoring system as read only properties, return named tuples for native properties, error handling for unsupported scoring systems --- archeryutils/rounds.py | 14 +++--- archeryutils/targets.py | 81 ++++++++++++++++-------------- archeryutils/tests/test_rounds.py | 4 +- archeryutils/tests/test_targets.py | 57 +++++++++++++++------ 4 files changed, 95 insertions(+), 61 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 6e9f21b..afc3d88 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -101,9 +101,9 @@ def __repr__(self) -> str: def __eq__(self, other: object) -> bool: """Check equality of Passes based on parameters.""" - if isinstance(other, Pass): - return self.n_arrows == other.n_arrows and self.target == other.target - return NotImplemented + if not isinstance(other, Pass): + return NotImplemented + return self.n_arrows == other.n_arrows and self.target == other.target @property def scoring_system(self) -> ScoringSystem: @@ -113,22 +113,22 @@ def scoring_system(self) -> ScoringSystem: @property def diameter(self) -> float: """Get target diameter [metres].""" - return self.target.diameter + return self.target._diameter # noqa: SLF001 @property def native_diameter_unit(self) -> str: """Get native_diameter_unit attribute of target.""" - return self.target.native_diameter_unit + return self.target._native_diameter_unit # noqa: SLF001 @property def distance(self) -> float: """Get target distance in [metres].""" - return self.target.distance + return self.target._distance # noqa: SLF001 @property def native_dist_unit(self) -> str: """Get native_dist_unit attribute of target.""" - return self.target.native_dist_unit + return self.target._native_dist_unit # noqa: SLF001 @property def indoor(self) -> bool: diff --git a/archeryutils/targets.py b/archeryutils/targets.py index db98dba..ba4cb5d 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -1,7 +1,7 @@ """Module for representing a Target for archery applications.""" from functools import partial -from typing import Literal, Optional, Union, get_args +from typing import Literal, NamedTuple, Optional, Union, get_args from archeryutils.constants import Length @@ -28,6 +28,13 @@ _rnd6 = partial(round, ndigits=6) +class Q(NamedTuple): + """Dataclass for a quantity with units.""" + + value: float + units: str + + class Target: """ Class to represent a target. @@ -117,11 +124,11 @@ def __init__( diameter, self.supported_diameter_units, "cm" ) - self.scoring_system = scoring_system - self.distance = Length.to_metres(dist, native_dist_unit) - self.native_dist_unit = native_dist_unit + self._scoring_system = scoring_system + self._distance = Length.to_metres(dist, native_dist_unit) + self._native_dist_unit = native_dist_unit self._diameter = Length.to_metres(diam, native_diameter_unit) - self.native_diameter_unit = native_diameter_unit + self._native_diameter_unit = native_diameter_unit self.indoor = indoor @classmethod @@ -206,45 +213,48 @@ def __eq__(self, other: object) -> bool: def _parameters(self): """Shortcut to get all target parameters as a tuple for comparison.""" return ( - self.scoring_system, - self.diameter, - self.native_diameter_unit, - self.distance, - self.native_dist_unit, + self._scoring_system, + self._diameter, + self._native_diameter_unit, + self._distance, + self._native_dist_unit, self.indoor, ) + @property + def scoring_system(self): + """Get target scoring system.""" + return self._scoring_system + @property def is_custom(self): """Check if this Target uses a custom scoring system.""" - return self.scoring_system == "Custom" + return self._scoring_system == "Custom" @property def diameter(self): - """Get target diameter.""" + """Get target diameter in [metres].""" return self._diameter - @diameter.setter - def diameter(self, value): - """Set target diameter and invalidate face spec for standard targets.""" - if self.scoring_system != "Custom": - self._face_spec = None - self._diameter = value + @property + def distance(self): + """Get target distance in [metres].""" + return self._distance @property - def native_distance(self) -> tuple[float, str]: + def native_distance(self) -> Q: """Get target distance in original native units.""" - return ( - Length.from_metres(self.distance, self.native_dist_unit), - self.native_dist_unit, + return Q( + Length.from_metres(self._distance, self._native_dist_unit), + self._native_dist_unit, ) @property - def native_diameter(self) -> tuple[float, str]: + def native_diameter(self) -> Q: """Get target diameter in original native units.""" - return ( - Length.from_metres(self._diameter, self.native_diameter_unit), - self.native_diameter_unit, + return Q( + Length.from_metres(self._diameter, self._native_diameter_unit), + self._native_diameter_unit, ) def max_score(self) -> float: @@ -256,11 +266,6 @@ def max_score(self) -> float: float maximum score possible on this target face. - Raises - ------ - ValueError - If a scoring system is not accounted for in the function. - Examples -------- >>> mytarget = au.Target("10_zone", (122, "cm"), (70.0, "m")) @@ -278,11 +283,6 @@ def min_score(self) -> float: float minimum score possible on this target face - Raises - ------ - ValueError - If a scoring system is not accounted for in the function. - Examples -------- >>> mytarget = au.Target("10_zone", (122, "cm"), (70.0, "m")) @@ -294,7 +294,14 @@ def min_score(self) -> float: @property def face_spec(self) -> FaceSpec: """Get the targets face specification, generating on demand if needed.""" - if self._face_spec is None and self.scoring_system != "Custom": + if self._face_spec is None: + if self.scoring_system == "Custom": + msg = ( + "Trying to generate face spec for custom target " + "but no existing spec found: " + "try instantiating with `Target.from_face_spec` instead" + ) + raise ValueError(msg) self._face_spec = self.gen_face_spec(self.scoring_system, self._diameter) return self._face_spec diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 882566a..e189932 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -88,14 +88,14 @@ def test_default_diameter_unit(self) -> None: test_pass = Pass.at_target(36, "5_zone", 122, 50) assert ( test_pass.native_diameter_unit - == test_pass.target.native_diameter_unit + == test_pass.target.native_diameter.units == "cm" ) def test_diameter_units_passed_to_target(self) -> None: """Check that Pass passes on diameter units to Target object.""" test_pass = Pass.at_target(60, "Worcester", (16, "inches"), (20, "yards")) - assert test_pass.target.native_diameter_unit == "inch" + assert test_pass.target.native_diameter.units == "inch" def test_default_location(self) -> None: """Check that Pass returns indoor=False when indoor not specified.""" diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index ab071e4..246856f 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -105,11 +105,12 @@ def test_invalid_distance_unit(self) -> None: def test_default_distance_unit(self) -> None: """Check that Target() returns distance in metres when units not specified.""" target = Target("5_zone", 122, 50) - assert target.native_dist_unit == "metre" + assert target.native_distance == (50, "metre") def test_yard_to_m_conversion(self) -> None: """Check Target() returns correct distance in metres when yards provided.""" target = Target("5_zone", 122, (50, "yards")) + assert target.native_distance == (50, "yard") assert target.distance == 50.0 * 0.9144 def test_invalid_diameter_unit(self) -> None: @@ -130,6 +131,7 @@ def test_diameter_metres_not_converted(self) -> None: def test_diameter_inches_supported(self) -> None: """Check that Target() converts diameters in inches correctly.""" target = Target("Worcester", (16, "inches"), (20, "yards"), indoor=True) + assert target.native_diameter == (16, "inch") assert target.diameter == 16 * 0.0254 def test_diameter_distance_units_coerced_to_definitive_names(self) -> None: @@ -146,10 +148,10 @@ def test_diameter_distance_units_coerced_to_definitive_names(self) -> None: (30, "Metres"), ) - assert imperial_target.native_dist_unit == "yard" - assert imperial_target.native_diameter_unit == "inch" - assert metric_target.native_dist_unit == "metre" - assert metric_target.native_diameter_unit == "cm" + assert imperial_target.native_distance.units == "yard" + assert imperial_target.native_diameter.units == "inch" + assert metric_target.native_distance.units == "metre" + assert metric_target.native_diameter.units == "cm" def test_default_location(self) -> None: """Check that Target() returns indoor=False when indoor not specified.""" @@ -188,7 +190,9 @@ def test_max_score_invalid_face_type(self) -> None: target = Target("5_zone", 122, 50, False) # Requires manual resetting of scoring system to get this error. # Silence mypy as scoring_system must be a valid literal ScoringSystem - target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] + # Ignore private attribute access to modify read only property + # Conclusion: no way this happens by accident + target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 target.max_score() @pytest.mark.parametrize( @@ -223,7 +227,9 @@ def test_min_score_invalid_face_type(self) -> None: target = Target("5_zone", 122, 50, False) # Requires manual resetting of scoring system to get this error. # Silence mypy as scoring_system must be a valid literal ScoringSystem - target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] + # Ignore private attribute access to modify read only property + # Conclusion: no way this happens by accident + target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 target.min_score() @pytest.mark.parametrize( @@ -319,20 +325,41 @@ def test_get_face_spec(self, scoring_system, diam, expected_spec) -> None: target = Target(scoring_system, diam, 30) assert target.face_spec == expected_spec - def test_get_face_spec_invalid_system(self) -> None: + def test_face_spec_wrong_constructor(self) -> None: + """ + Accessing face spec raises an error for custom target from standard init. + + Custom targets should be made using the `from_face_spec` classmethod + """ + target = Target("Custom", 122, 50) + with pytest.raises( + ValueError, + match=( + "Trying to generate face spec for custom target " + "but no existing spec found" + ), + ): + assert target.face_spec + + def test_face_spec_invalid_system(self) -> None: """Check error is raised when trying to get specs of an unsupported system.""" target = Target("5_zone", 122, 50) # Silence mypy as scoring_system must be a valid literal ScoringSystem - target.scoring_system = "InvalidScoringSystem" # type: ignore[assignment] + # Ignore private attribute access to modify read only property + # Conclusion: no way this happens by accident + target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): assert target.face_spec - def test_face_spec_recaluclated_after_changing_diameter(self) -> None: - """Check that face_spec respects the current target diameter.""" - target = Target("10_zone_5_ring", 80, 50) - assert target.face_spec == {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6} - target.diameter /= 2 - assert target.face_spec == {0.04: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} + # def test_face_spec_recaluclated_after_changing_diameter(self) -> None: + # """Check that face_spec respects the current target diameter.""" + # target = Target("10_zone_5_ring", 80, 50) + # assert target.face_spec == {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6} + # target.diameter /= 2 + # assert target.face_spec == {0.04: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} + + # def test_mutating_diameter_units(self) -> None: + # pass class TestCustomScoringTarget: From cdd6033ead645508dbf29f80064da79fb4011a54 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Fri, 8 Mar 2024 09:42:44 +0000 Subject: [PATCH 17/43] Clear up type hinting for FaceSpec, make read_only --- archeryutils/handicaps/handicap_scheme.py | 4 ++-- archeryutils/targets.py | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index 4135bd0..019a0a9 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -234,13 +234,13 @@ def arrow_score( return self._s_bar(spec, arw_rad, sig_r) def _s_bar( - self, target_specs: dict[float, int], arw_rad: float, sig_r: FloatArray + self, target_specs: targets.FaceSpec, arw_rad: float, sig_r: FloatArray ) -> FloatArray: """Calculate expected score directly from target ring sizes. Parameters ---------- - target_specs : dict[float, int] + target_specs : FaceSpec Mapping of target ring *diameters* in [metres], to points scored arw_rad : float arrow radius in [metres] diff --git a/archeryutils/targets.py b/archeryutils/targets.py index ba4cb5d..48d8525 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -1,6 +1,8 @@ """Module for representing a Target for archery applications.""" +from collections.abc import Mapping from functools import partial +from types import MappingProxyType from typing import Literal, NamedTuple, Optional, Union, get_args from archeryutils.constants import Length @@ -23,7 +25,7 @@ ] # TypeAlias (annotate explicitly in py3.10+) -FaceSpec = dict[float, int] +FaceSpec = Mapping[float, int] _rnd6 = partial(round, ndigits=6) @@ -303,7 +305,7 @@ def face_spec(self) -> FaceSpec: ) raise ValueError(msg) self._face_spec = self.gen_face_spec(self.scoring_system, self._diameter) - return self._face_spec + return MappingProxyType(self._face_spec) @staticmethod def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: From c4d6110926a554dc0b2bb0f034e0d05d778f5e4f Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Fri, 8 Mar 2024 11:01:48 +0000 Subject: [PATCH 18/43] Increased test coverage of Length utilities --- archeryutils/tests/test_constants.py | 49 ++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/archeryutils/tests/test_constants.py b/archeryutils/tests/test_constants.py index 8f32fa8..d92b45b 100644 --- a/archeryutils/tests/test_constants.py +++ b/archeryutils/tests/test_constants.py @@ -65,3 +65,52 @@ def test_conversion_from_metres(self, value, unit, result): def test_unit_name_coercion(self, unit, result): """Test unit name standardisation available on Length class.""" assert Length.definitive_unit(unit) == result + + def test_unit_alias_reduction(self): + """Test full set of unit alises can be reduced to just definitive names.""" + assert Length.definitive_units(Length.inch | Length.cm) == {"inch", "cm"} + + @pytest.mark.parametrize( + "value,expected", + [ + pytest.param( + 10, + (10, "metre"), + id="int-scalar", + ), + pytest.param( + 10.1, + (10.1, "metre"), + id="float-scalar", + ), + pytest.param( + (10, "Metres"), + (10, "metre"), + id="default-units", + ), + pytest.param( + (10, "yds"), + (10, "yard"), + id="other-units", + ), + ], + ) + def test_optional_unit_parsing(self, value, expected): + """Test parsing of quantities with and without units.""" + supported = Length.metre | Length.yard + default = "metre" + assert Length.parse_optional_units(value, supported, default) == expected + + def test_optional_unit_parsing_units_not_supported(self): + """Test parsing of quantities with and without units.""" + with pytest.raises(ValueError, match="Unit (.+) not recognised. Select from"): + assert Length.parse_optional_units( + (10, "bannana"), Length.metre | Length.yard, "metre" + ) + + def test_optional_unit_parsing_default_not_supported(self): + """Test parsing of quantities with and without units.""" + with pytest.raises( + ValueError, match="Default unit (.+) must be in supported units" + ): + assert Length.parse_optional_units(10, Length.metre | Length.yard, "inch") From 6fa21af20fd486c78fa599974ac7d66318a16ca5 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Fri, 8 Mar 2024 12:35:32 +0000 Subject: [PATCH 19/43] Refactor Length classmethods to instance methods and provide instance to export --- archeryutils/constants.py | 58 ++++++++++++++++++---------- archeryutils/rounds.py | 4 +- archeryutils/targets.py | 23 ++++++----- archeryutils/tests/test_constants.py | 36 ++++++++--------- 4 files changed, 68 insertions(+), 53 deletions(-) diff --git a/archeryutils/constants.py b/archeryutils/constants.py index fc8f7f9..cab1aa4 100644 --- a/archeryutils/constants.py +++ b/archeryutils/constants.py @@ -1,5 +1,6 @@ """Constants used in the archeryutils package.""" +from collections.abc import Collection, Set from typing import ClassVar, TypeVar, Union T = TypeVar("T") @@ -67,15 +68,17 @@ class Length: Contains common abbreviations, pluralisations and capitilizations for supported units as sets to allow easy membership checks in combination. - Methods for conversions to and from metres are provided as classmethods. - + Supported units are provided as class attributes for easy autocompletion """ + # Add alias to any new supported units here yard = _YARD_ALIASES metre = _METRE_ALIASES cm = _CM_ALIASES inch = _INCH_ALIASES + # Update _ALIASES and _CONVERSIONS_TO_M for any new supported units + # And they will be automatically incorporated here _reversed: ClassVar = { alias: name for name in _CONVERSIONS_TO_M for alias in _ALIASES[name] } @@ -86,8 +89,12 @@ class Length: for alias in _ALIASES[name] } - @classmethod - def to_metres(cls, value: float, unit: str) -> float: + @property + def known_units(self): + """Display all units that can be converted by this class.""" + return tuple(self.definitive_units(self._conversions)) + + def to_metres(self, value: float, unit: str) -> float: """ Convert distance in any supported unit to metres. @@ -108,10 +115,9 @@ def to_metres(cls, value: float, unit: str) -> float: >>> Length.to_metres(10, "inches") 0.254 """ - return cls._conversions[unit] * value + return self._conversions[unit] * value - @classmethod - def from_metres(cls, metre_value: float, unit: str) -> float: + def from_metres(self, metre_value: float, unit: str) -> float: """ Convert distance in metres to specified unit. @@ -132,10 +138,9 @@ def from_metres(cls, metre_value: float, unit: str) -> float: >>> Length.from_metres(18.3, "yards") 20.0131 """ - return metre_value / cls._conversions[unit] + return metre_value / self._conversions[unit] - @classmethod - def definitive_unit(cls, alias: str) -> str: + def definitive_unit(self, alias: str) -> str: """ Convert any string alias representing a distance unit to a single version. @@ -154,10 +159,9 @@ def definitive_unit(cls, alias: str) -> str: >>> Length.definitive_unit("Metres") "metre" """ - return cls._reversed[alias] + return self._reversed[alias] - @classmethod - def definitive_units(cls, aliases: set[str]) -> set[str]: + def definitive_units(self, aliases: Collection[str]) -> set[str]: """ Reduce a set of string unit aliases to just their definitive names. @@ -176,13 +180,12 @@ def definitive_units(cls, aliases: set[str]) -> set[str]: >>> Length.definitive_unit(Length.metre | Length.yard) {'metre', 'yard'} """ - return {cls._reversed[alias] for alias in aliases} + return {self._reversed[alias] for alias in aliases} - @classmethod def parse_optional_units( - cls, + self, value: Union[T, tuple[T, str]], - supported: set[str], + supported: Set[str], default: str, ) -> tuple[T, str]: """ @@ -193,9 +196,9 @@ def parse_optional_units( Parameters ---------- value : Any or tuple of Any, str - Either a single object, or a tuple with the desired units + Either a single object, or a tuple with the desired units. supported: set of str - Valid unit aliases to be accepted + Set of units (and aliases) that are expected/supported. default: str Default unit to be used when value does not specify units. @@ -210,6 +213,16 @@ def parse_optional_units( tuple of Any, str original value, definitive name of unit + Notes + ----- + The supported parameter encodes both which units can be used, + and also which aliases are acceptable for those units. + If your downstream functionality for example only works with imperial units. + Then pass supported = {'inch', 'inches', 'yard', 'yards'} etc. + The units parsed from value will be checked against this set. + The default parameter is what will be provided in the result if the input value + is a scalar, so this must also be present in the set of supported units. + Examples -------- >>> m_and_yd = Length.metre | Length.yard @@ -232,7 +245,10 @@ def parse_optional_units( if units not in supported: msg = ( f"Unit {units!r} not recognised. " - f"Select from {cls.definitive_units(supported)}." + f"Select from {self.definitive_units(supported)}." ) raise ValueError(msg) - return quantity, cls.definitive_unit(units) + return quantity, self.definitive_unit(units) + + +length = Length() diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index afc3d88..646796d 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -3,7 +3,7 @@ from collections.abc import Iterable from typing import Optional, Union -from archeryutils.constants import Length +from archeryutils.constants import length from archeryutils.targets import ScoringSystem, Target @@ -258,7 +258,7 @@ def max_distance(self, unit: bool = False) -> Union[float, tuple[float, str]]: max_dist = pass_i.distance d_unit = pass_i.native_dist_unit - max_dist = Length.from_metres(max_dist, d_unit) + max_dist = length.from_metres(max_dist, d_unit) if unit: return (max_dist, d_unit) return max_dist diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 48d8525..ea5de86 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -5,7 +5,7 @@ from types import MappingProxyType from typing import Literal, NamedTuple, Optional, Union, get_args -from archeryutils.constants import Length +from archeryutils.constants import length # TypeAlias (annotate explicitly in py3.10+) ScoringSystem = Literal[ @@ -103,8 +103,8 @@ class Target: _face_spec: Optional[FaceSpec] = None supported_systems = get_args(ScoringSystem) - supported_distance_units = Length.yard | Length.metre - supported_diameter_units = Length.cm | Length.inch | Length.metre + supported_distance_units = length.yard | length.metre + supported_diameter_units = length.cm | length.inch | length.metre def __init__( self, @@ -119,17 +119,16 @@ def __init__( f"""Please select from '{"', '".join(self.supported_systems)}'.""" ) raise ValueError(msg) - dist, native_dist_unit = Length.parse_optional_units( + dist, native_dist_unit = length.parse_optional_units( distance, self.supported_distance_units, "metre" ) - diam, native_diameter_unit = Length.parse_optional_units( + diam, native_diameter_unit = length.parse_optional_units( diameter, self.supported_diameter_units, "cm" ) - self._scoring_system = scoring_system - self._distance = Length.to_metres(dist, native_dist_unit) + self._distance = length.to_metres(dist, native_dist_unit) self._native_dist_unit = native_dist_unit - self._diameter = Length.to_metres(diam, native_diameter_unit) + self._diameter = length.to_metres(diam, native_diameter_unit) self._native_diameter_unit = native_diameter_unit self.indoor = indoor @@ -175,11 +174,11 @@ def from_face_spec( >>> specs = {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} >>> target = Target.from_spec(specs, 40, 18) """ - spec_data, spec_units = Length.parse_optional_units( + spec_data, spec_units = length.parse_optional_units( face_spec, cls.supported_diameter_units, "metre" ) spec = { - _rnd6(Length.to_metres(ring_diam, spec_units)): score + _rnd6(length.to_metres(ring_diam, spec_units)): score for ring_diam, score in spec_data.items() } @@ -247,7 +246,7 @@ def distance(self): def native_distance(self) -> Q: """Get target distance in original native units.""" return Q( - Length.from_metres(self._distance, self._native_dist_unit), + length.from_metres(self._distance, self._native_dist_unit), self._native_dist_unit, ) @@ -255,7 +254,7 @@ def native_distance(self) -> Q: def native_diameter(self) -> Q: """Get target diameter in original native units.""" return Q( - Length.from_metres(self._diameter, self._native_diameter_unit), + length.from_metres(self._diameter, self._native_diameter_unit), self._native_diameter_unit, ) diff --git a/archeryutils/tests/test_constants.py b/archeryutils/tests/test_constants.py index d92b45b..e9cd83e 100644 --- a/archeryutils/tests/test_constants.py +++ b/archeryutils/tests/test_constants.py @@ -2,7 +2,7 @@ import pytest -from archeryutils.constants import Length +from archeryutils.constants import length CM = "cm" INCH = "inch" @@ -15,17 +15,17 @@ class TestLengths: def test_units_available(self): """Test common length unit names.""" - assert CM in Length.cm - assert INCH in Length.inch - assert METRE in Length.metre - assert YARD in Length.yard + assert CM in length.cm + assert INCH in length.inch + assert METRE in length.metre + assert YARD in length.yard def test_pluralised_unit_alises_available(self): """Test plurailised versions of common length unit names.""" - assert CM + "s" in Length.cm - assert INCH + "es" in Length.inch - assert METRE + "s" in Length.metre - assert YARD + "s" in Length.yard + assert CM + "s" in length.cm + assert INCH + "es" in length.inch + assert METRE + "s" in length.metre + assert YARD + "s" in length.yard @pytest.mark.parametrize( "value,unit,result", @@ -38,7 +38,7 @@ def test_pluralised_unit_alises_available(self): ) def test_conversion_to_metres(self, value, unit, result): """Test conversion from other units to metres.""" - assert Length.to_metres(value, unit) == result + assert length.to_metres(value, unit) == result @pytest.mark.parametrize( "value,unit,result", @@ -51,7 +51,7 @@ def test_conversion_to_metres(self, value, unit, result): ) def test_conversion_from_metres(self, value, unit, result): """Test conversion from metres to other units.""" - assert Length.from_metres(value, unit) == pytest.approx(result) + assert length.from_metres(value, unit) == pytest.approx(result) @pytest.mark.parametrize( "unit,result", @@ -64,11 +64,11 @@ def test_conversion_from_metres(self, value, unit, result): ) def test_unit_name_coercion(self, unit, result): """Test unit name standardisation available on Length class.""" - assert Length.definitive_unit(unit) == result + assert length.definitive_unit(unit) == result def test_unit_alias_reduction(self): """Test full set of unit alises can be reduced to just definitive names.""" - assert Length.definitive_units(Length.inch | Length.cm) == {"inch", "cm"} + assert length.definitive_units(length.inch | length.cm) == {"inch", "cm"} @pytest.mark.parametrize( "value,expected", @@ -97,15 +97,15 @@ def test_unit_alias_reduction(self): ) def test_optional_unit_parsing(self, value, expected): """Test parsing of quantities with and without units.""" - supported = Length.metre | Length.yard + supported = length.metre | length.yard default = "metre" - assert Length.parse_optional_units(value, supported, default) == expected + assert length.parse_optional_units(value, supported, default) == expected def test_optional_unit_parsing_units_not_supported(self): """Test parsing of quantities with and without units.""" with pytest.raises(ValueError, match="Unit (.+) not recognised. Select from"): - assert Length.parse_optional_units( - (10, "bannana"), Length.metre | Length.yard, "metre" + assert length.parse_optional_units( + (10, "bannana"), length.metre | length.yard, "metre" ) def test_optional_unit_parsing_default_not_supported(self): @@ -113,4 +113,4 @@ def test_optional_unit_parsing_default_not_supported(self): with pytest.raises( ValueError, match="Default unit (.+) must be in supported units" ): - assert Length.parse_optional_units(10, Length.metre | Length.yard, "inch") + assert length.parse_optional_units(10, length.metre | length.yard, "inch") From 47b9c25b7d9aa455322331d5e52b52a8a00bb784 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 13:07:21 +0000 Subject: [PATCH 20/43] Enforce default 0 values for min and max score Handles special case where empty spec passed to custom target, is consitent with implmentation of _s_bar which returns an average score of 0 in such cases --- archeryutils/targets.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index ea5de86..9d139da 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -273,7 +273,7 @@ def max_score(self) -> float: >>> mytarget.max_score() 10.0 """ - return max(self.face_spec.values()) + return max(self.face_spec.values(), default=0) def min_score(self) -> float: """ @@ -290,7 +290,7 @@ def min_score(self) -> float: >>> mytarget.min_score() 1.0 """ - return min(self.face_spec.values()) + return min(self.face_spec.values(), default=0) @property def face_spec(self) -> FaceSpec: From 91536c9c40d6fbb4f6b5baea8b645e99de048c16 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 13:59:20 +0000 Subject: [PATCH 21/43] Refactor face_spec property to be automatically set on initialisation for all targets Can remove tests for deliberate mutation of private _scoring_system after initialisation as the behaviour is now undefined. Simplifies logic in face_spec getter property and removes checks from the happy path. Re raises an error in case the user accidently tries to create a custom scoring target via the regular constructor. --- archeryutils/targets.py | 35 ++++++++++++++----------- archeryutils/tests/test_targets.py | 42 ------------------------------ 2 files changed, 20 insertions(+), 57 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 9d139da..889abcd 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -3,7 +3,7 @@ from collections.abc import Mapping from functools import partial from types import MappingProxyType -from typing import Literal, NamedTuple, Optional, Union, get_args +from typing import Literal, NamedTuple, Union, get_args from archeryutils.constants import length @@ -100,7 +100,7 @@ class Target: """ - _face_spec: Optional[FaceSpec] = None + _face_spec: FaceSpec supported_systems = get_args(ScoringSystem) supported_distance_units = length.yard | length.metre @@ -132,6 +132,9 @@ def __init__( self._native_diameter_unit = native_diameter_unit self.indoor = indoor + if scoring_system != "Custom": + self._face_spec = self.gen_face_spec(scoring_system, self._diameter) + @classmethod def from_face_spec( cls, @@ -295,16 +298,19 @@ def min_score(self) -> float: @property def face_spec(self) -> FaceSpec: """Get the targets face specification, generating on demand if needed.""" - if self._face_spec is None: - if self.scoring_system == "Custom": - msg = ( - "Trying to generate face spec for custom target " - "but no existing spec found: " - "try instantiating with `Target.from_face_spec` instead" - ) - raise ValueError(msg) - self._face_spec = self.gen_face_spec(self.scoring_system, self._diameter) - return MappingProxyType(self._face_spec) + # Still have some error handling in here for the case where + # users use the wrong initaliser: + # eg target = Target("Custom", 10, 10) + # As otherwise errors raised are somewhat cryptic + try: + return MappingProxyType(self._face_spec) + except AttributeError as err: + msg = ( + "Trying to generate face spec for custom target " + "but no existing spec found: " + "try instantiating with `Target.from_face_spec` instead" + ) + raise ValueError(msg) from err @staticmethod def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: @@ -357,9 +363,8 @@ def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: spec = {_rnd6(n * diameter / 5): 6 - n for n in range(1, 6 - missing)} # NB: Should be hard (but not impossible) to get here without catching earlier; - # Can only occur if target scoring system is modified after initialisation - # Or newly supported scoring system doesn't have an implementation - # here for generating specs + # Most likely will only occur if a newly supported scoring system doesn't + # have an implementation here for generating specs else: msg = f"Scoring system {system!r} is not supported" raise ValueError(msg) diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 246856f..2f5ea89 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -184,17 +184,6 @@ def test_max_score( target = Target(face_type, 122, (50, "metre"), False) assert target.max_score() == max_score_expected - def test_max_score_invalid_face_type(self) -> None: - """Check that Target() raises error for invalid face.""" - with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): - target = Target("5_zone", 122, 50, False) - # Requires manual resetting of scoring system to get this error. - # Silence mypy as scoring_system must be a valid literal ScoringSystem - # Ignore private attribute access to modify read only property - # Conclusion: no way this happens by accident - target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 - target.max_score() - @pytest.mark.parametrize( "face_type,min_score_expected", [ @@ -221,17 +210,6 @@ def test_min_score( target = Target(face_type, 122, 50, False) assert target.min_score() == min_score_expected - def test_min_score_invalid_face_type(self) -> None: - """Check that Target() raises error for invalid face.""" - with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): - target = Target("5_zone", 122, 50, False) - # Requires manual resetting of scoring system to get this error. - # Silence mypy as scoring_system must be a valid literal ScoringSystem - # Ignore private attribute access to modify read only property - # Conclusion: no way this happens by accident - target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 - target.min_score() - @pytest.mark.parametrize( "scoring_system, diam, expected_spec", [ @@ -341,26 +319,6 @@ def test_face_spec_wrong_constructor(self) -> None: ): assert target.face_spec - def test_face_spec_invalid_system(self) -> None: - """Check error is raised when trying to get specs of an unsupported system.""" - target = Target("5_zone", 122, 50) - # Silence mypy as scoring_system must be a valid literal ScoringSystem - # Ignore private attribute access to modify read only property - # Conclusion: no way this happens by accident - target._scoring_system = "InvalidScoringSystem" # type: ignore[assignment] # noqa: SLF001 - with pytest.raises(ValueError, match="Scoring system '(.+)' is not supported"): - assert target.face_spec - - # def test_face_spec_recaluclated_after_changing_diameter(self) -> None: - # """Check that face_spec respects the current target diameter.""" - # target = Target("10_zone_5_ring", 80, 50) - # assert target.face_spec == {0.08: 10, 0.16: 9, 0.24: 8, 0.32: 7, 0.4: 6} - # target.diameter /= 2 - # assert target.face_spec == {0.04: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} - - # def test_mutating_diameter_units(self) -> None: - # pass - class TestCustomScoringTarget: """Tests for Target class with custom scoring.""" From 939da6cd9054cfae8af48f52a80cfc33d6e4b03d Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 15:33:15 +0000 Subject: [PATCH 22/43] Refactor target properties to store native lengths as Quantity tuples instead of just native units --- archeryutils/rounds.py | 16 ++++++++-------- archeryutils/targets.py | 32 +++++++++++++------------------- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 646796d..1948a73 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -113,22 +113,22 @@ def scoring_system(self) -> ScoringSystem: @property def diameter(self) -> float: """Get target diameter [metres].""" - return self.target._diameter # noqa: SLF001 - - @property - def native_diameter_unit(self) -> str: - """Get native_diameter_unit attribute of target.""" - return self.target._native_diameter_unit # noqa: SLF001 + return self.target.diameter @property def distance(self) -> float: """Get target distance in [metres].""" - return self.target._distance # noqa: SLF001 + return self.target.distance + + @property + def native_diameter_unit(self) -> str: + """Get native_diameter_unit attribute of target.""" + return self.target.native_diameter.units @property def native_dist_unit(self) -> str: """Get native_dist_unit attribute of target.""" - return self.target._native_dist_unit # noqa: SLF001 + return self.target.native_distance.units @property def indoor(self) -> bool: diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 889abcd..fa1a530 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -30,7 +30,7 @@ _rnd6 = partial(round, ndigits=6) -class Q(NamedTuple): +class Quantity(NamedTuple): """Dataclass for a quantity with units.""" value: float @@ -122,14 +122,14 @@ def __init__( dist, native_dist_unit = length.parse_optional_units( distance, self.supported_distance_units, "metre" ) - diam, native_diameter_unit = length.parse_optional_units( + diam, native_diam_unit = length.parse_optional_units( diameter, self.supported_diameter_units, "cm" ) self._scoring_system = scoring_system self._distance = length.to_metres(dist, native_dist_unit) - self._native_dist_unit = native_dist_unit - self._diameter = length.to_metres(diam, native_diameter_unit) - self._native_diameter_unit = native_diameter_unit + self._native_distance = Quantity(dist, native_dist_unit) + self._diameter = length.to_metres(diam, native_diam_unit) + self._native_diameter = Quantity(diam, native_diam_unit) self.indoor = indoor if scoring_system != "Custom": @@ -219,9 +219,9 @@ def _parameters(self): return ( self._scoring_system, self._diameter, - self._native_diameter_unit, + self._native_diameter, self._distance, - self._native_dist_unit, + self._native_distance, self.indoor, ) @@ -236,30 +236,24 @@ def is_custom(self): return self._scoring_system == "Custom" @property - def diameter(self): + def diameter(self) -> float: """Get target diameter in [metres].""" return self._diameter @property - def distance(self): + def distance(self) -> float: """Get target distance in [metres].""" return self._distance @property - def native_distance(self) -> Q: + def native_distance(self) -> Quantity: """Get target distance in original native units.""" - return Q( - length.from_metres(self._distance, self._native_dist_unit), - self._native_dist_unit, - ) + return self._native_distance @property - def native_diameter(self) -> Q: + def native_diameter(self) -> Quantity: """Get target diameter in original native units.""" - return Q( - length.from_metres(self._diameter, self._native_diameter_unit), - self._native_diameter_unit, - ) + return self._native_diameter def max_score(self) -> float: """ From ca8585c133d307be71a8b86d09b0960545c418e5 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 15:46:57 +0000 Subject: [PATCH 23/43] Simplify equality check and remove unused is_custom computed property --- archeryutils/targets.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index fa1a530..8eef4c9 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -207,11 +207,6 @@ def __eq__(self, other: object) -> bool: if not isinstance(other, Target): return NotImplemented - if self.scoring_system == other.scoring_system == "Custom": - return ( - self._face_spec == other._face_spec - and self._parameters() == other._parameters() - ) return self._parameters() == other._parameters() def _parameters(self): @@ -223,6 +218,7 @@ def _parameters(self): self._distance, self._native_distance, self.indoor, + self.face_spec, ) @property @@ -230,11 +226,6 @@ def scoring_system(self): """Get target scoring system.""" return self._scoring_system - @property - def is_custom(self): - """Check if this Target uses a custom scoring system.""" - return self._scoring_system == "Custom" - @property def diameter(self) -> float: """Get target diameter in [metres].""" From 00e584de0155ef0fc566878f11e79777688993e2 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 16:24:41 +0000 Subject: [PATCH 24/43] Refactor pass properties and methods to match new target properties. All pass properties are now a straight passthrough to the corresponding matching target properties. Round.max_distance refactored to replace manual iteration with native max(), and use new pass properties. --- archeryutils/rounds.py | 31 +++++++++++++++---------------- archeryutils/tests/test_rounds.py | 10 +++++----- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 1948a73..d99fc44 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -4,7 +4,7 @@ from typing import Optional, Union from archeryutils.constants import length -from archeryutils.targets import ScoringSystem, Target +from archeryutils.targets import Quantity, ScoringSystem, Target class Pass: @@ -121,14 +121,14 @@ def distance(self) -> float: return self.target.distance @property - def native_diameter_unit(self) -> str: + def native_diameter(self) -> Quantity: """Get native_diameter_unit attribute of target.""" - return self.target.native_diameter.units + return self.target.native_diameter @property - def native_dist_unit(self) -> str: + def native_distance(self) -> Quantity: """Get native_dist_unit attribute of target.""" - return self.target.native_distance.units + return self.target.native_distance @property def indoor(self) -> bool: @@ -251,17 +251,16 @@ def max_distance(self, unit: bool = False) -> Union[float, tuple[float, str]]: maximum distance shot in this round (max_dist, unit) : tuple (float, str) tuple of max_dist and string of unit - """ - max_dist = 0.0 - for pass_i in self.passes: - if pass_i.distance > max_dist: - max_dist = pass_i.distance - d_unit = pass_i.native_dist_unit - max_dist = length.from_metres(max_dist, d_unit) + Notes + ----- + This does not convert the units of the result. + Rather the maximum distance shot in the round is returned in + """ + longest_pass = max(self.passes, key=lambda p: p.distance) if unit: - return (max_dist, d_unit) - return max_dist + return longest_pass.native_distance + return longest_pass.native_distance.value def get_info(self) -> None: """ @@ -273,8 +272,8 @@ def get_info(self) -> None: """ print(f"A {self.name} consists of {len(self.passes)} passes:") for pass_i in self.passes: - diam, diam_units = pass_i.target.native_diameter - dist, dist_units = pass_i.target.native_distance + diam, diam_units = pass_i.native_diameter + dist, dist_units = pass_i.native_distance print( f"\t- {pass_i.n_arrows} arrows " f"at a {diam:.1f} {diam_units} target " diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index e189932..9652799 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -81,13 +81,13 @@ def test_equality(self, other, result) -> None: def test_default_distance_unit(self) -> None: """Check that Pass returns distance in metres when units not specified.""" test_pass = Pass.at_target(36, "5_zone", 122, 50) - assert test_pass.native_dist_unit == "metre" + assert test_pass.native_distance.units == "metre" def test_default_diameter_unit(self) -> None: """Check that Pass has same default diameter units as Target.""" test_pass = Pass.at_target(36, "5_zone", 122, 50) assert ( - test_pass.native_diameter_unit + test_pass.native_diameter.units == test_pass.target.native_diameter.units == "cm" ) @@ -111,17 +111,17 @@ def test_properties(self) -> None: """Check that Pass properties are set correctly.""" test_pass = Pass(36, Target("5_zone", (122, "cm"), (50, "metre"), False)) assert test_pass.distance == 50.0 - assert test_pass.native_dist_unit == "metre" + assert test_pass.native_distance == (50, "metre") assert test_pass.diameter == 1.22 assert test_pass.scoring_system == "5_zone" assert test_pass.indoor is False - assert test_pass.native_diameter_unit == "cm" + assert test_pass.native_diameter == (122, "cm") def test_custom_target(self) -> None: """Check that pass can be constructed from a custom target specification.""" target = Target.from_face_spec({0.1: 3, 0.5: 1}, 80, (50, "yard")) test_pass = Pass(30, target) - assert test_pass.target.is_custom + assert test_pass @pytest.mark.parametrize( "face_type,max_score_expected", From e7b8860735090e244c8d0c90a2fee01c5a38abd3 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 19:12:17 +0000 Subject: [PATCH 25/43] Rename load_rounds.custom to load_rounds.misc to avoid confusion with custom scoring targets --- archeryutils/load_rounds.py | 2 +- .../round_data_files/{Custom.json => Miscellaneous.json} | 0 docs/api/archeryutils.preloaded_rounds.rst | 4 ++-- docs/getting-started/quickstart.rst | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename archeryutils/round_data_files/{Custom.json => Miscellaneous.json} (100%) diff --git a/archeryutils/load_rounds.py b/archeryutils/load_rounds.py index a3a0c51..609d591 100644 --- a/archeryutils/load_rounds.py +++ b/archeryutils/load_rounds.py @@ -188,7 +188,7 @@ def _make_rounds_dict(json_name: str) -> DotDict: IFAA_field = _make_rounds_dict("IFAA_field.json") WA_VI = _make_rounds_dict("WA_VI.json") AGB_VI = _make_rounds_dict("AGB_VI.json") -custom = _make_rounds_dict("Custom.json") +misc = _make_rounds_dict("Miscellaneous.json") del _make_rounds_dict diff --git a/archeryutils/round_data_files/Custom.json b/archeryutils/round_data_files/Miscellaneous.json similarity index 100% rename from archeryutils/round_data_files/Custom.json rename to archeryutils/round_data_files/Miscellaneous.json diff --git a/docs/api/archeryutils.preloaded_rounds.rst b/docs/api/archeryutils.preloaded_rounds.rst index e060b76..474a71b 100644 --- a/docs/api/archeryutils.preloaded_rounds.rst +++ b/docs/api/archeryutils.preloaded_rounds.rst @@ -120,11 +120,11 @@ AGB VI au.load_rounds.AGB_VI -Custom Rounds +Miscellaneous ------------- .. ipython:: python import archeryutils as au - au.load_rounds.custom + au.load_rounds.misc diff --git a/docs/getting-started/quickstart.rst b/docs/getting-started/quickstart.rst index e7b11c0..0cb902a 100644 --- a/docs/getting-started/quickstart.rst +++ b/docs/getting-started/quickstart.rst @@ -142,7 +142,7 @@ Possible options for round collections are: * ``IFAA_field`` - IFAA indoor and outdoor rounds * ``AGB_VI`` - Archery GB Visually Impaired rounds * ``WA_VI`` - World Archery Visually Impaired rounds -* ``custom`` - custom rounds such as individual distances, 252 awards, frostbites etc. +* ``misc`` - Miscellaneous rounds such as individual distances, 252 awards, frostbites etc. Handicap Schemes ---------------- From b057829c8f7765747cdeb27e6b203fc4b5e83a1f Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sat, 9 Mar 2024 20:32:14 +0000 Subject: [PATCH 26/43] Documentation for custom scoring targets. Reorder source code and docstrings to be consistent in ordering of diameter and distance with function signature. --- archeryutils/rounds.py | 4 +- archeryutils/targets.py | 94 ++++++++++++++++++--------- docs/api/archeryutils.baseclasses.rst | 6 ++ docs/conf.py | 1 + docs/getting-started/quickstart.rst | 36 +++++++++- examples.ipynb | 41 ++++++++++++ 6 files changed, 148 insertions(+), 34 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index d99fc44..6dc95ec 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -3,7 +3,6 @@ from collections.abc import Iterable from typing import Optional, Union -from archeryutils.constants import length from archeryutils.targets import Quantity, ScoringSystem, Target @@ -236,7 +235,7 @@ def max_score(self) -> float: """ return sum(pass_i.max_score() for pass_i in self.passes) - def max_distance(self, unit: bool = False) -> Union[float, tuple[float, str]]: + def max_distance(self, unit: bool = False) -> Union[float, Quantity]: """ Return the maximum distance shot on this round along with the unit (optional). @@ -256,6 +255,7 @@ def max_distance(self, unit: bool = False) -> Union[float, tuple[float, str]]: ----- This does not convert the units of the result. Rather the maximum distance shot in the round is returned in + whatever units it was defined in. """ longest_pass = max(self.passes, key=lambda p: p.distance) if unit: diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 8eef4c9..eb00880 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -31,7 +31,18 @@ class Quantity(NamedTuple): - """Dataclass for a quantity with units.""" + """ + Dataclass for a quantity with units. + + Can be used in place of a plain tuple of (value, units) + + Attributes + ---------- + value: float + Scalar value of quantity + units: str + Units of quantity + """ value: float units: str @@ -62,12 +73,12 @@ class Target: target face/scoring system type. diameter : float Target face diameter [metres]. - native_diameter_unit : str - Native unit the target size is measured in before conversion to [metres]. distance : float linear distance from archer to target [metres]. - native_dist_unit : str - Native unit the target distance is measured in before conversion to [metres]. + native_diameter : Quantity + Native target diameter and unit before conversion to [metres]. + native_distance : Quantity + Native target distance and unit before conversion to [metres]. indoor : bool, default=False is round indoors? @@ -119,17 +130,17 @@ def __init__( f"""Please select from '{"', '".join(self.supported_systems)}'.""" ) raise ValueError(msg) - dist, native_dist_unit = length.parse_optional_units( - distance, self.supported_distance_units, "metre" - ) diam, native_diam_unit = length.parse_optional_units( diameter, self.supported_diameter_units, "cm" ) + dist, native_dist_unit = length.parse_optional_units( + distance, self.supported_distance_units, "metre" + ) self._scoring_system = scoring_system - self._distance = length.to_metres(dist, native_dist_unit) - self._native_distance = Quantity(dist, native_dist_unit) self._diameter = length.to_metres(diam, native_diam_unit) self._native_diameter = Quantity(diam, native_diam_unit) + self._distance = length.to_metres(dist, native_dist_unit) + self._native_distance = Quantity(dist, native_dist_unit) self.indoor = indoor if scoring_system != "Custom": @@ -173,9 +184,10 @@ def from_face_spec( Examples -------- - >>> # WA 18m compound triple spot - >>> specs = {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} - >>> target = Target.from_spec(specs, 40, 18) + >>> # Kings of archery recurve scoring triple spot + >>> specs = {0.08: 10, 0.12: 8, 0.16: 7, 0.2: 6} + >>> target = Target.from_face_spec(specs, 40, 18) + >>> assert target.scoring_system == "Custom" """ spec_data, spec_units = length.parse_optional_units( face_spec, cls.supported_diameter_units, "metre" @@ -246,6 +258,31 @@ def native_diameter(self) -> Quantity: """Get target diameter in original native units.""" return self._native_diameter + @property + def face_spec(self) -> FaceSpec: + """ + Get the targets face specification. + + Raises + ------ + ValueError + If trying to access the face_spec for a `"Custom"` scoring target + but that target was not instantiated correctly and no spec is found. + """ + # Still have some error handling in here for the case where + # users use the wrong initaliser: + # eg target = Target("Custom", 10, 10) + # As otherwise errors raised are somewhat cryptic + try: + return MappingProxyType(self._face_spec) + except AttributeError as err: + msg = ( + "Trying to generate face spec for custom target " + "but no existing spec found: " + "try instantiating with `Target.from_face_spec` instead" + ) + raise ValueError(msg) from err + def max_score(self) -> float: """ Return the maximum numerical score possible on this target (i.e. not X). @@ -280,23 +317,6 @@ def min_score(self) -> float: """ return min(self.face_spec.values(), default=0) - @property - def face_spec(self) -> FaceSpec: - """Get the targets face specification, generating on demand if needed.""" - # Still have some error handling in here for the case where - # users use the wrong initaliser: - # eg target = Target("Custom", 10, 10) - # As otherwise errors raised are somewhat cryptic - try: - return MappingProxyType(self._face_spec) - except AttributeError as err: - msg = ( - "Trying to generate face spec for custom target " - "but no existing spec found: " - "try instantiating with `Target.from_face_spec` instead" - ) - raise ValueError(msg) from err - @staticmethod def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: """ @@ -307,12 +327,24 @@ def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: system: ScoringSystem Name of scoring system diameter: - Target diameter in metres + Target diameter in [metres] Returns ------- spec : dict Mapping of target ring sizes in [metres] to score + + Raises + ------ + ValueError + If no rule for producing a face_spec from the given system is found. + + Examples + -------- + >>> Target.gen_face_spec("WA_field", 0.6) + {0.06: 6, 0.12: 5, 0.24: 4, 0.36: 3, 0.48: 2, 0.6: 1} + >>> Target.gen_face_spec("10_zone_5_ring_compound", 0.4) + {0.02: 10, 0.08: 9, 0.12: 8, 0.16: 7, 0.2: 6} """ removed_rings = { "10_zone_6_ring": 4, diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index d47a545..9cce0e2 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -1,10 +1,16 @@ Base Classes ============ +.. autoclass:: archeryutils.targets.Quantity .. autoclass:: archeryutils.Target :members: :undoc-members: :show-inheritance: +.. + Getting warnings for duplicate links being generated for class properties + that are also referenced in the class docstring. + Eg distance, diameter. + This is beyond my limited sphinx experience here. .. autoclass:: archeryutils.Pass :members: diff --git a/docs/conf.py b/docs/conf.py index 4f324c1..1c3dec5 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -45,3 +45,4 @@ html_theme = "sphinx_rtd_theme" html_static_path = ["_static"] +autodoc_member_order = "bysource" diff --git a/docs/getting-started/quickstart.rst b/docs/getting-started/quickstart.rst index 0cb902a..f745f69 100644 --- a/docs/getting-started/quickstart.rst +++ b/docs/getting-started/quickstart.rst @@ -60,11 +60,45 @@ as a tuple: ) myIFAATarget = au.Target("IFAA_field", diameter=80, distance=(80.0, "yards")) +If the target you want is not supported, you can manually supply the target ring sizes +and scores as a `FaceSpec` and construct a target as so. + +.. ipython:: python + + # Kings of archery recurve scoring target + face_spec = {8: 10, 12: 8, 16: 7, 20: 6} + myKingsTarget = au.Target.from_face_spec((face_spec, "cm"), 40, 18, indoor=True) + print(myKingsTarget.scoring_system) + +.. note:: + Although we can provide the face_spec in any unit listed under :py:attr:`~archeryutils.Target.supported_diameter_units`, + the sizes in the specification are converted to metres and stored in this form. + Therefore unlike the target diameter paramater, the default unit for specifications is [metres] + +The only limitations to the target faces you can represent in this way are: + +1. Targets must be formed of concentric rings +2. The score must monotonically decrease as the rings get larger + +Under the hood, all standard scoring systems autogenerate their own `FaceSpec` and this is used +internally when calculating handicaps and classifications. You can see this stored under the +:py:attr:`~archeryutils.Target.face_spec` property: + +.. ipython:: python + + print(my720target.face_spec) + The target features `max_score()` and `min_score()` methods: .. ipython:: python - for target in [my720target, mycompound720target, myIFAATarget, myWorcesterTarget]: + for target in [ + my720target, + mycompound720target, + myIFAATarget, + myWorcesterTarget, + myKingsTarget, + ]: print( f"{target.scoring_system} has max score {target.max_score()} ", f"and min score {target.min_score()}.", diff --git a/examples.ipynb b/examples.ipynb index a6233c4..c978f82 100644 --- a/examples.ipynb +++ b/examples.ipynb @@ -134,6 +134,47 @@ "myIFAATarget = au.Target(\"IFAA_field\", diameter=80, distance=(80.0, \"yards\"))" ] }, + { + "cell_type": "markdown", + "id": "5227ae9b", + "metadata": {}, + "source": [ + "Sometimes you might want to represent a target that isn't represented by the built in scoring systems.\n", + "In this case you can manually supply the target ring sizes and scores as a `FaceSpec`, which a mapping (commonly a dict) of ring diameters to scores\n", + "and construct a target as so:" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "b64c8ced", + "metadata": {}, + "outputs": [], + "source": [ + "# Kings of archery recurve scoring target\n", + "face_spec = {8: 10, 12: 8, 16: 7, 20: 6}\n", + "myKingsTarget = au.Target.from_face_spec((face_spec, \"cm\"), 40, 18, indoor=True)\n", + "print(myKingsTarget.scoring_system)" + ] + }, + { + "cell_type": "markdown", + "id": "f44592a2", + "metadata": {}, + "source": [ + "Under the hood, all standard scoring systems autogenerate their own `FaceSpec` and this is used internally when calculating handicaps and classifications. You can see this stored under the `Target.face_spec` property:" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "c113e587", + "metadata": {}, + "outputs": [], + "source": [ + "print(my720target.face_spec)" + ] + }, { "cell_type": "markdown", "id": "c46e1fda", From ad568a62aa24fb9386a5373ec1a9ce43f6a27a93 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sun, 10 Mar 2024 08:19:40 +0000 Subject: [PATCH 27/43] Test length.known_units convinience property and simplify return type --- archeryutils/constants.py | 4 ++-- archeryutils/tests/test_constants.py | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/archeryutils/constants.py b/archeryutils/constants.py index cab1aa4..25c3a4f 100644 --- a/archeryutils/constants.py +++ b/archeryutils/constants.py @@ -90,9 +90,9 @@ class Length: } @property - def known_units(self): + def known_units(self) -> set[str]: """Display all units that can be converted by this class.""" - return tuple(self.definitive_units(self._conversions)) + return self.definitive_units(self._conversions) def to_metres(self, value: float, unit: str) -> float: """ diff --git a/archeryutils/tests/test_constants.py b/archeryutils/tests/test_constants.py index e9cd83e..84f01dc 100644 --- a/archeryutils/tests/test_constants.py +++ b/archeryutils/tests/test_constants.py @@ -14,6 +14,10 @@ class TestLengths: """Tests for Length class.""" def test_units_available(self): + """Check and document currently supported units.""" + assert length.known_units == {CM, INCH, METRE, YARD} + + def test_units_available_on_attributes(self): """Test common length unit names.""" assert CM in length.cm assert INCH in length.inch From 9cba392dbd58172b41d7ca33631b64dbded314b8 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Sun, 10 Mar 2024 08:29:42 +0000 Subject: [PATCH 28/43] Test case to cover generating face spec with invalid scoring system --- archeryutils/tests/test_targets.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/archeryutils/tests/test_targets.py b/archeryutils/tests/test_targets.py index 2f5ea89..83cfac2 100644 --- a/archeryutils/tests/test_targets.py +++ b/archeryutils/tests/test_targets.py @@ -298,7 +298,7 @@ def test_min_score( ), ], ) - def test_get_face_spec(self, scoring_system, diam, expected_spec) -> None: + def test_face_spec(self, scoring_system, diam, expected_spec) -> None: """Check that target returns face specs from supported scoring systems.""" target = Target(scoring_system, diam, 30) assert target.face_spec == expected_spec @@ -319,6 +319,15 @@ def test_face_spec_wrong_constructor(self) -> None: ): assert target.face_spec + def test_gen_face_spec_unsupported_system(self) -> None: + """Check that generating face spec for an unsupported system raises error.""" + with pytest.raises( + ValueError, + match="Scoring system '(.+)' is not supported", + ): + # Silence mypy as using known invalid scoring system for test + assert Target.gen_face_spec("Dartchery", 100) # type: ignore[arg-type] + class TestCustomScoringTarget: """Tests for Target class with custom scoring.""" From fa93843eccc18b5106cfa7400e251032a6822b31 Mon Sep 17 00:00:00 2001 From: Jack Atkinson Date: Sun, 10 Mar 2024 14:01:03 +0000 Subject: [PATCH 29/43] Clean Target docstrings to remove old attributes that are now properties causing Sphinx to display duplicates. Also make internal data on supported systems and units private. --- archeryutils/targets.py | 30 +++++++++------------------ docs/api/archeryutils.baseclasses.rst | 5 ----- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index eb00880..88419af 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -69,16 +69,6 @@ class Target: Attributes ---------- - scoring_system : ScoringSystem - target face/scoring system type. - diameter : float - Target face diameter [metres]. - distance : float - linear distance from archer to target [metres]. - native_diameter : Quantity - Native target diameter and unit before conversion to [metres]. - native_distance : Quantity - Native target distance and unit before conversion to [metres]. indoor : bool, default=False is round indoors? @@ -113,9 +103,9 @@ class Target: _face_spec: FaceSpec - supported_systems = get_args(ScoringSystem) - supported_distance_units = length.yard | length.metre - supported_diameter_units = length.cm | length.inch | length.metre + _supported_systems = get_args(ScoringSystem) + _supported_distance_units = length.yard | length.metre + _supported_diameter_units = length.cm | length.inch | length.metre def __init__( self, @@ -124,17 +114,17 @@ def __init__( distance: Union[float, tuple[float, str]], indoor: bool = False, ) -> None: - if scoring_system not in self.supported_systems: + if scoring_system not in self._supported_systems: msg = ( f"""Invalid Target Face Type specified.\n""" - f"""Please select from '{"', '".join(self.supported_systems)}'.""" + f"""Please select from '{"', '".join(self._supported_systems)}'.""" ) raise ValueError(msg) diam, native_diam_unit = length.parse_optional_units( - diameter, self.supported_diameter_units, "cm" + diameter, self._supported_diameter_units, "cm" ) dist, native_dist_unit = length.parse_optional_units( - distance, self.supported_distance_units, "metre" + distance, self._supported_distance_units, "metre" ) self._scoring_system = scoring_system self._diameter = length.to_metres(diam, native_diam_unit) @@ -190,7 +180,7 @@ def from_face_spec( >>> assert target.scoring_system == "Custom" """ spec_data, spec_units = length.parse_optional_units( - face_spec, cls.supported_diameter_units, "metre" + face_spec, cls._supported_diameter_units, "metre" ) spec = { _rnd6(length.to_metres(ring_diam, spec_units)): score @@ -234,8 +224,8 @@ def _parameters(self): ) @property - def scoring_system(self): - """Get target scoring system.""" + def scoring_system(self) -> ScoringSystem: + """Get the target face/scoring system type.""" return self._scoring_system @property diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index 9cce0e2..0b6b0ff 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -6,11 +6,6 @@ Base Classes :members: :undoc-members: :show-inheritance: -.. - Getting warnings for duplicate links being generated for class properties - that are also referenced in the class docstring. - Eg distance, diameter. - This is beyond my limited sphinx experience here. .. autoclass:: archeryutils.Pass :members: From c5d496377cb00c1127f99d549c225a9368f58831 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 11 Mar 2024 08:30:31 +0000 Subject: [PATCH 30/43] Refactor Length class to module and rename to convert Avoids confusion of initialising single instance and passing around. All important data for calculating conversions and aliases was already stored as global constants in the module, so now just use these directly from functions instead of incorporating into class --- archeryutils/constants.py | 254 ----------------- archeryutils/convert.py | 258 ++++++++++++++++++ archeryutils/targets.py | 19 +- .../{test_constants.py => test_convert.py} | 40 +-- 4 files changed, 289 insertions(+), 282 deletions(-) delete mode 100644 archeryutils/constants.py create mode 100644 archeryutils/convert.py rename archeryutils/tests/{test_constants.py => test_convert.py} (73%) diff --git a/archeryutils/constants.py b/archeryutils/constants.py deleted file mode 100644 index 25c3a4f..0000000 --- a/archeryutils/constants.py +++ /dev/null @@ -1,254 +0,0 @@ -"""Constants used in the archeryutils package.""" - -from collections.abc import Collection, Set -from typing import ClassVar, TypeVar, Union - -T = TypeVar("T") - -_CONVERSIONS_TO_M = { - "metre": 1.0, - "yard": 0.9144, - "cm": 0.01, - "inch": 0.0254, -} - -_YARD_ALIASES = { - "Yard", - "yard", - "Yards", - "yards", - "Y", - "y", - "Yd", - "yd", - "Yds", - "yds", -} - -_METRE_ALIASES = { - "Metre", - "metre", - "Metres", - "metres", - "M", - "m", - "Ms", - "ms", -} - -_CM_ALIASES = { - "Centimetre", - "centimetre", - "Centimetres", - "centimetres", - "CM", - "cm", - "CMs", - "cms", -} - -_INCH_ALIASES = { - "Inch", - "inch", - "Inches", - "inches", -} - -_ALIASES = { - "yard": _YARD_ALIASES, - "metre": _METRE_ALIASES, - "cm": _CM_ALIASES, - "inch": _INCH_ALIASES, -} - - -class Length: - """ - Utility class for Length unit conversions. - - Contains common abbreviations, pluralisations and capitilizations for supported - units as sets to allow easy membership checks in combination. - Supported units are provided as class attributes for easy autocompletion - """ - - # Add alias to any new supported units here - yard = _YARD_ALIASES - metre = _METRE_ALIASES - cm = _CM_ALIASES - inch = _INCH_ALIASES - - # Update _ALIASES and _CONVERSIONS_TO_M for any new supported units - # And they will be automatically incorporated here - _reversed: ClassVar = { - alias: name for name in _CONVERSIONS_TO_M for alias in _ALIASES[name] - } - - _conversions: ClassVar = { - alias: factor - for name, factor in _CONVERSIONS_TO_M.items() - for alias in _ALIASES[name] - } - - @property - def known_units(self) -> set[str]: - """Display all units that can be converted by this class.""" - return self.definitive_units(self._conversions) - - def to_metres(self, value: float, unit: str) -> float: - """ - Convert distance in any supported unit to metres. - - Parameters - ---------- - value : float - scalar value of distance to be converted to metres - unit : str - units of distance to be converted to metres - - Returns - ------- - float - scalar value of converted distance in metres - - Examples - -------- - >>> Length.to_metres(10, "inches") - 0.254 - """ - return self._conversions[unit] * value - - def from_metres(self, metre_value: float, unit: str) -> float: - """ - Convert distance in metres to specified unit. - - Parameters - ---------- - metre_value : float - scalar value of distance in metres to be converted - unit : str - units distance is to be converted TO - - Returns - ------- - float - scalar value of converted distance in the provided unit - - Examples - -------- - >>> Length.from_metres(18.3, "yards") - 20.0131 - """ - return metre_value / self._conversions[unit] - - def definitive_unit(self, alias: str) -> str: - """ - Convert any string alias representing a distance unit to a single version. - - Parameters - ---------- - alias : str - name of unit to be converted - - Returns - ------- - str - definitive name of unit - - Examples - -------- - >>> Length.definitive_unit("Metres") - "metre" - """ - return self._reversed[alias] - - def definitive_units(self, aliases: Collection[str]) -> set[str]: - """ - Reduce a set of string unit aliases to just their definitive names. - - Parameters - ---------- - aliases : set of str - names of units to be converted - - Returns - ------- - set of str - definitive names of unit - - Examples - -------- - >>> Length.definitive_unit(Length.metre | Length.yard) - {'metre', 'yard'} - """ - return {self._reversed[alias] for alias in aliases} - - def parse_optional_units( - self, - value: Union[T, tuple[T, str]], - supported: Set[str], - default: str, - ) -> tuple[T, str]: - """ - Parse single value or tuple of value and units. - - Always returns a tuple of value and units - - Parameters - ---------- - value : Any or tuple of Any, str - Either a single object, or a tuple with the desired units. - supported: set of str - Set of units (and aliases) that are expected/supported. - default: str - Default unit to be used when value does not specify units. - - Raises - ------ - ValueError - If default units or parsed values units - are not contained in supported units. - - Returns - ------- - tuple of Any, str - original value, definitive name of unit - - Notes - ----- - The supported parameter encodes both which units can be used, - and also which aliases are acceptable for those units. - If your downstream functionality for example only works with imperial units. - Then pass supported = {'inch', 'inches', 'yard', 'yards'} etc. - The units parsed from value will be checked against this set. - The default parameter is what will be provided in the result if the input value - is a scalar, so this must also be present in the set of supported units. - - Examples - -------- - >>> m_and_yd = Length.metre | Length.yard - >>> Length.parse_optional_units(10, m_and_yd, "metre") - (10, 'metre') - >>> Length.parse_optional_units((10, 'yards') m_and_yd, 'metre') - (10, 'yard') - >>> Length.parse_optional_units((10, 'banana') m_and_yd, 'metre') - ValueError: Unit 'banana' not recognised. Select from {'yard', 'metre'}. - """ - if default not in supported: - msg = f"Default unit {default!r} must be in supported units" - raise ValueError(msg) - if isinstance(value, tuple) and len(value) == 2: # noqa: PLR2004 - quantity, units = value - else: - quantity = value - units = default - - if units not in supported: - msg = ( - f"Unit {units!r} not recognised. " - f"Select from {self.definitive_units(supported)}." - ) - raise ValueError(msg) - return quantity, self.definitive_unit(units) - - -length = Length() diff --git a/archeryutils/convert.py b/archeryutils/convert.py new file mode 100644 index 0000000..630ec57 --- /dev/null +++ b/archeryutils/convert.py @@ -0,0 +1,258 @@ +"""Utility module for conversion of qunatities and unit aliases. + +Contains common abbreviations, pluralisations and capitilizations for supported +units as sets to allow easy membership checks in combination. +Supported units are provided as module attributes for easy autocompletion, +""" + +from collections import abc as _abc +from typing import TypeVar as _TypeVar +from typing import Union as _Union + +__all__ = [ + "yard", + "metre", + "inch", + "cm", + "to_metres", + "from_metres", + "definitive_unit", + "definitive_units", + "parse_optional_units", + "known_units", +] + +_T = _TypeVar("_T") + +# Add aliases to any new supported units here +yard = { + "Yard", + "yard", + "Yards", + "yards", + "Y", + "y", + "Yd", + "yd", + "Yds", + "yds", +} + +metre = { + "Metre", + "metre", + "Metres", + "metres", + "M", + "m", + "Ms", + "ms", +} + +cm = { + "Centimetre", + "centimetre", + "Centimetres", + "centimetres", + "CM", + "cm", + "CMs", + "cms", +} + +inch = { + "Inch", + "inch", + "Inches", + "inches", +} + +# Update _ALIASES and _CONVERSIONS_TO_M for any new supported units +# And they will be automatically incorporated +_ALIASES = { + "yard": yard, + "metre": metre, + "cm": cm, + "inch": inch, +} + + +_CONVERSIONS_TO_M = { + "metre": 1.0, + "yard": 0.9144, + "cm": 0.01, + "inch": 0.0254, +} + + +_reversed = {alias: name for name in _CONVERSIONS_TO_M for alias in _ALIASES[name]} + +_conversions = { + alias: factor + for name, factor in _CONVERSIONS_TO_M.items() + for alias in _ALIASES[name] +} + + +def to_metres(value: float, unit: str) -> float: + """ + Convert distance in any supported unit to metres. + + Parameters + ---------- + value : float + scalar value of distance to be converted to metres + unit : str + units of distance to be converted to metres + + Returns + ------- + float + scalar value of converted distance in metres + + Examples + -------- + >>> convert.to_metres(10, "inches") + 0.254 + """ + return _conversions[unit] * value + + +def from_metres(metre_value: float, unit: str) -> float: + """ + Convert distance in metres to specified unit. + + Parameters + ---------- + metre_value : float + scalar value of distance in metres to be converted + unit : str + units distance is to be converted TO + + Returns + ------- + float + scalar value of converted distance in the provided unit + + Examples + -------- + >>> convert.from_metres(18.3, "yards") + 20.0131 + """ + return metre_value / _conversions[unit] + + +def definitive_unit(alias: str) -> str: + """ + Convert any string alias representing a distance unit to a single version. + + Parameters + ---------- + alias : str + name of unit to be converted + + Returns + ------- + str + definitive name of unit + + Examples + -------- + >>> convert.definitive_unit("Metres") + "metre" + """ + return _reversed[alias] + + +def definitive_units(aliases: _abc.Collection[str]) -> set[str]: + """ + Reduce a set of string unit aliases to just their definitive names. + + Parameters + ---------- + aliases : set of str + names of units to be converted + + Returns + ------- + set of str + definitive names of unit + + Examples + -------- + >>> convert.definitive_units(convert.metre | convert.yard) + {'metre', 'yard'} + """ + return {_reversed[alias] for alias in aliases} + + +def parse_optional_units( + value: _Union[_T, tuple[_T, str]], + supported: _abc.Set[str], + default: str, +) -> tuple[_T, str]: + """ + Parse single value or tuple of value and units. + + Always returns a tuple of value and units + + Parameters + ---------- + value : Any or tuple of Any, str + Either a single object, or a tuple with the desired units. + supported: set of str + Set of units (and aliases) that are expected/supported. + default: str + Default unit to be used when value does not specify units. + + Raises + ------ + ValueError + If default units or parsed values units + are not contained in supported units. + + Returns + ------- + tuple of Any, str + original value, definitive name of unit + + Notes + ----- + The supported parameter encodes both which units can be used, + and also which aliases are acceptable for those units. + If your downstream functionality for example only works with imperial units. + Then pass supported = {'inch', 'inches', 'yard', 'yards'} etc. + The units parsed from value will be checked against this set. + The default parameter is what will be provided in the result if the input value + is a scalar, so this must also be present in the set of supported units. + + Examples + -------- + >>> m_and_yd = convert.metre | convert.yard + >>> convert.parse_optional_units(10, m_and_yd, "metre") + (10, 'metre') + >>> convert.parse_optional_units((10, 'yards') m_and_yd, 'metre') + (10, 'yard') + >>> convert.parse_optional_units((10, 'banana') m_and_yd, 'metre') + ValueError: Unit 'banana' not recognised. Select from {'yard', 'metre'}. + """ + if default not in supported: + msg = f"Default unit {default!r} must be in supported units" + raise ValueError(msg) + if isinstance(value, tuple) and len(value) == 2: # noqa: PLR2004 + quantity, units = value + else: + quantity = value + units = default + + if units not in supported: + msg = ( + f"Unit {units!r} not recognised. " + f"Select from {definitive_units(supported)}." + ) + raise ValueError(msg) + return quantity, definitive_unit(units) + + +known_units: set[str] = definitive_units(_conversions) +"""Display all units that can be converted by this module.""" diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 88419af..2ac61be 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -5,7 +5,7 @@ from types import MappingProxyType from typing import Literal, NamedTuple, Union, get_args -from archeryutils.constants import length +from archeryutils import convert # TypeAlias (annotate explicitly in py3.10+) ScoringSystem = Literal[ @@ -104,8 +104,8 @@ class Target: _face_spec: FaceSpec _supported_systems = get_args(ScoringSystem) - _supported_distance_units = length.yard | length.metre - _supported_diameter_units = length.cm | length.inch | length.metre + _supported_distance_units = convert.yard | convert.metre + _supported_diameter_units = convert.cm | convert.inch | convert.metre def __init__( self, @@ -120,16 +120,17 @@ def __init__( f"""Please select from '{"', '".join(self._supported_systems)}'.""" ) raise ValueError(msg) - diam, native_diam_unit = length.parse_optional_units( + + diam, native_diam_unit = convert.parse_optional_units( diameter, self._supported_diameter_units, "cm" ) - dist, native_dist_unit = length.parse_optional_units( + dist, native_dist_unit = convert.parse_optional_units( distance, self._supported_distance_units, "metre" ) self._scoring_system = scoring_system - self._diameter = length.to_metres(diam, native_diam_unit) + self._diameter = convert.to_metres(diam, native_diam_unit) self._native_diameter = Quantity(diam, native_diam_unit) - self._distance = length.to_metres(dist, native_dist_unit) + self._distance = convert.to_metres(dist, native_dist_unit) self._native_distance = Quantity(dist, native_dist_unit) self.indoor = indoor @@ -179,11 +180,11 @@ def from_face_spec( >>> target = Target.from_face_spec(specs, 40, 18) >>> assert target.scoring_system == "Custom" """ - spec_data, spec_units = length.parse_optional_units( + spec_data, spec_units = convert.parse_optional_units( face_spec, cls._supported_diameter_units, "metre" ) spec = { - _rnd6(length.to_metres(ring_diam, spec_units)): score + _rnd6(convert.to_metres(ring_diam, spec_units)): score for ring_diam, score in spec_data.items() } diff --git a/archeryutils/tests/test_constants.py b/archeryutils/tests/test_convert.py similarity index 73% rename from archeryutils/tests/test_constants.py rename to archeryutils/tests/test_convert.py index 84f01dc..d256ba8 100644 --- a/archeryutils/tests/test_constants.py +++ b/archeryutils/tests/test_convert.py @@ -2,7 +2,7 @@ import pytest -from archeryutils.constants import length +from archeryutils import convert CM = "cm" INCH = "inch" @@ -15,21 +15,21 @@ class TestLengths: def test_units_available(self): """Check and document currently supported units.""" - assert length.known_units == {CM, INCH, METRE, YARD} + assert convert.known_units == {CM, INCH, METRE, YARD} def test_units_available_on_attributes(self): """Test common length unit names.""" - assert CM in length.cm - assert INCH in length.inch - assert METRE in length.metre - assert YARD in length.yard + assert CM in convert.cm + assert INCH in convert.inch + assert METRE in convert.metre + assert YARD in convert.yard def test_pluralised_unit_alises_available(self): """Test plurailised versions of common length unit names.""" - assert CM + "s" in length.cm - assert INCH + "es" in length.inch - assert METRE + "s" in length.metre - assert YARD + "s" in length.yard + assert CM + "s" in convert.cm + assert INCH + "es" in convert.inch + assert METRE + "s" in convert.metre + assert YARD + "s" in convert.yard @pytest.mark.parametrize( "value,unit,result", @@ -42,7 +42,7 @@ def test_pluralised_unit_alises_available(self): ) def test_conversion_to_metres(self, value, unit, result): """Test conversion from other units to metres.""" - assert length.to_metres(value, unit) == result + assert convert.to_metres(value, unit) == result @pytest.mark.parametrize( "value,unit,result", @@ -55,7 +55,7 @@ def test_conversion_to_metres(self, value, unit, result): ) def test_conversion_from_metres(self, value, unit, result): """Test conversion from metres to other units.""" - assert length.from_metres(value, unit) == pytest.approx(result) + assert convert.from_metres(value, unit) == pytest.approx(result) @pytest.mark.parametrize( "unit,result", @@ -68,11 +68,11 @@ def test_conversion_from_metres(self, value, unit, result): ) def test_unit_name_coercion(self, unit, result): """Test unit name standardisation available on Length class.""" - assert length.definitive_unit(unit) == result + assert convert.definitive_unit(unit) == result def test_unit_alias_reduction(self): """Test full set of unit alises can be reduced to just definitive names.""" - assert length.definitive_units(length.inch | length.cm) == {"inch", "cm"} + assert convert.definitive_units(convert.inch | convert.cm) == {"inch", "cm"} @pytest.mark.parametrize( "value,expected", @@ -101,15 +101,15 @@ def test_unit_alias_reduction(self): ) def test_optional_unit_parsing(self, value, expected): """Test parsing of quantities with and without units.""" - supported = length.metre | length.yard + supported = convert.metre | convert.yard default = "metre" - assert length.parse_optional_units(value, supported, default) == expected + assert convert.parse_optional_units(value, supported, default) == expected def test_optional_unit_parsing_units_not_supported(self): """Test parsing of quantities with and without units.""" with pytest.raises(ValueError, match="Unit (.+) not recognised. Select from"): - assert length.parse_optional_units( - (10, "bannana"), length.metre | length.yard, "metre" + assert convert.parse_optional_units( + (10, "bannana"), convert.metre | convert.yard, "metre" ) def test_optional_unit_parsing_default_not_supported(self): @@ -117,4 +117,6 @@ def test_optional_unit_parsing_default_not_supported(self): with pytest.raises( ValueError, match="Default unit (.+) must be in supported units" ): - assert length.parse_optional_units(10, length.metre | length.yard, "inch") + assert convert.parse_optional_units( + 10, convert.metre | convert.yard, "inch" + ) From 88438c1d712f2cd89a2567b0ffe5e49ef157f9fc Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 11 Mar 2024 12:58:36 +0000 Subject: [PATCH 31/43] Revert name aliasing typing imports, rename convert back to length --- archeryutils/{convert.py => length.py} | 23 +++++----- archeryutils/targets.py | 18 ++++---- .../tests/{test_convert.py => test_length.py} | 44 +++++++++---------- 3 files changed, 42 insertions(+), 43 deletions(-) rename archeryutils/{convert.py => length.py} (91%) rename archeryutils/tests/{test_convert.py => test_length.py} (71%) diff --git a/archeryutils/convert.py b/archeryutils/length.py similarity index 91% rename from archeryutils/convert.py rename to archeryutils/length.py index 630ec57..8c50e60 100644 --- a/archeryutils/convert.py +++ b/archeryutils/length.py @@ -5,9 +5,8 @@ Supported units are provided as module attributes for easy autocompletion, """ -from collections import abc as _abc -from typing import TypeVar as _TypeVar -from typing import Union as _Union +from collections.abc import Collection, Set +from typing import TypeVar, Union __all__ = [ "yard", @@ -22,7 +21,7 @@ "known_units", ] -_T = _TypeVar("_T") +T = TypeVar("T") # Add aliases to any new supported units here yard = { @@ -67,7 +66,7 @@ "inches", } -# Update _ALIASES and _CONVERSIONS_TO_M for any new supported units +# Update _ALIASES and _CONVERSIONSTO_M for any new supported units # And they will be automatically incorporated _ALIASES = { "yard": yard, @@ -77,7 +76,7 @@ } -_CONVERSIONS_TO_M = { +_CONVERSIONSTO_M = { "metre": 1.0, "yard": 0.9144, "cm": 0.01, @@ -85,11 +84,11 @@ } -_reversed = {alias: name for name in _CONVERSIONS_TO_M for alias in _ALIASES[name]} +_reversed = {alias: name for name in _CONVERSIONSTO_M for alias in _ALIASES[name]} _conversions = { alias: factor - for name, factor in _CONVERSIONS_TO_M.items() + for name, factor in _CONVERSIONSTO_M.items() for alias in _ALIASES[name] } @@ -164,7 +163,7 @@ def definitive_unit(alias: str) -> str: return _reversed[alias] -def definitive_units(aliases: _abc.Collection[str]) -> set[str]: +def definitive_units(aliases: Collection[str]) -> set[str]: """ Reduce a set of string unit aliases to just their definitive names. @@ -187,10 +186,10 @@ def definitive_units(aliases: _abc.Collection[str]) -> set[str]: def parse_optional_units( - value: _Union[_T, tuple[_T, str]], - supported: _abc.Set[str], + value: Union[T, tuple[T, str]], + supported: Set[str], default: str, -) -> tuple[_T, str]: +) -> tuple[T, str]: """ Parse single value or tuple of value and units. diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 2ac61be..a4bb939 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -5,7 +5,7 @@ from types import MappingProxyType from typing import Literal, NamedTuple, Union, get_args -from archeryutils import convert +from archeryutils import length # TypeAlias (annotate explicitly in py3.10+) ScoringSystem = Literal[ @@ -104,8 +104,8 @@ class Target: _face_spec: FaceSpec _supported_systems = get_args(ScoringSystem) - _supported_distance_units = convert.yard | convert.metre - _supported_diameter_units = convert.cm | convert.inch | convert.metre + _supported_distance_units = length.yard | length.metre + _supported_diameter_units = length.cm | length.inch | length.metre def __init__( self, @@ -121,16 +121,16 @@ def __init__( ) raise ValueError(msg) - diam, native_diam_unit = convert.parse_optional_units( + diam, native_diam_unit = length.parse_optional_units( diameter, self._supported_diameter_units, "cm" ) - dist, native_dist_unit = convert.parse_optional_units( + dist, native_dist_unit = length.parse_optional_units( distance, self._supported_distance_units, "metre" ) self._scoring_system = scoring_system - self._diameter = convert.to_metres(diam, native_diam_unit) + self._diameter = length.to_metres(diam, native_diam_unit) self._native_diameter = Quantity(diam, native_diam_unit) - self._distance = convert.to_metres(dist, native_dist_unit) + self._distance = length.to_metres(dist, native_dist_unit) self._native_distance = Quantity(dist, native_dist_unit) self.indoor = indoor @@ -180,11 +180,11 @@ def from_face_spec( >>> target = Target.from_face_spec(specs, 40, 18) >>> assert target.scoring_system == "Custom" """ - spec_data, spec_units = convert.parse_optional_units( + spec_data, spec_units = length.parse_optional_units( face_spec, cls._supported_diameter_units, "metre" ) spec = { - _rnd6(convert.to_metres(ring_diam, spec_units)): score + _rnd6(length.to_metres(ring_diam, spec_units)): score for ring_diam, score in spec_data.items() } diff --git a/archeryutils/tests/test_convert.py b/archeryutils/tests/test_length.py similarity index 71% rename from archeryutils/tests/test_convert.py rename to archeryutils/tests/test_length.py index d256ba8..26499ed 100644 --- a/archeryutils/tests/test_convert.py +++ b/archeryutils/tests/test_length.py @@ -1,8 +1,8 @@ -"""Tests for constants, including Length class.""" +"""Tests for length conversion module.""" import pytest -from archeryutils import convert +from archeryutils import length CM = "cm" INCH = "inch" @@ -11,25 +11,25 @@ class TestLengths: - """Tests for Length class.""" + """Tests for length module.""" def test_units_available(self): """Check and document currently supported units.""" - assert convert.known_units == {CM, INCH, METRE, YARD} + assert length.known_units == {CM, INCH, METRE, YARD} def test_units_available_on_attributes(self): """Test common length unit names.""" - assert CM in convert.cm - assert INCH in convert.inch - assert METRE in convert.metre - assert YARD in convert.yard + assert CM in length.cm + assert INCH in length.inch + assert METRE in length.metre + assert YARD in length.yard def test_pluralised_unit_alises_available(self): """Test plurailised versions of common length unit names.""" - assert CM + "s" in convert.cm - assert INCH + "es" in convert.inch - assert METRE + "s" in convert.metre - assert YARD + "s" in convert.yard + assert CM + "s" in length.cm + assert INCH + "es" in length.inch + assert METRE + "s" in length.metre + assert YARD + "s" in length.yard @pytest.mark.parametrize( "value,unit,result", @@ -42,7 +42,7 @@ def test_pluralised_unit_alises_available(self): ) def test_conversion_to_metres(self, value, unit, result): """Test conversion from other units to metres.""" - assert convert.to_metres(value, unit) == result + assert length.to_metres(value, unit) == result @pytest.mark.parametrize( "value,unit,result", @@ -55,7 +55,7 @@ def test_conversion_to_metres(self, value, unit, result): ) def test_conversion_from_metres(self, value, unit, result): """Test conversion from metres to other units.""" - assert convert.from_metres(value, unit) == pytest.approx(result) + assert length.from_metres(value, unit) == pytest.approx(result) @pytest.mark.parametrize( "unit,result", @@ -68,11 +68,11 @@ def test_conversion_from_metres(self, value, unit, result): ) def test_unit_name_coercion(self, unit, result): """Test unit name standardisation available on Length class.""" - assert convert.definitive_unit(unit) == result + assert length.definitive_unit(unit) == result def test_unit_alias_reduction(self): """Test full set of unit alises can be reduced to just definitive names.""" - assert convert.definitive_units(convert.inch | convert.cm) == {"inch", "cm"} + assert length.definitive_units(length.inch | length.cm) == {"inch", "cm"} @pytest.mark.parametrize( "value,expected", @@ -101,15 +101,15 @@ def test_unit_alias_reduction(self): ) def test_optional_unit_parsing(self, value, expected): """Test parsing of quantities with and without units.""" - supported = convert.metre | convert.yard + supported = length.metre | length.yard default = "metre" - assert convert.parse_optional_units(value, supported, default) == expected + assert length.parse_optional_units(value, supported, default) == expected def test_optional_unit_parsing_units_not_supported(self): """Test parsing of quantities with and without units.""" with pytest.raises(ValueError, match="Unit (.+) not recognised. Select from"): - assert convert.parse_optional_units( - (10, "bannana"), convert.metre | convert.yard, "metre" + assert length.parse_optional_units( + (10, "bannana"), length.metre | length.yard, "metre" ) def test_optional_unit_parsing_default_not_supported(self): @@ -117,6 +117,6 @@ def test_optional_unit_parsing_default_not_supported(self): with pytest.raises( ValueError, match="Default unit (.+) must be in supported units" ): - assert convert.parse_optional_units( - 10, convert.metre | convert.yard, "inch" + assert length.parse_optional_units( + 10, length.metre | length.yard, "inch" ) From e9ba3ea98eadc87c3ad5494f67e5bb9f573c2df4 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 11 Mar 2024 13:31:58 +0000 Subject: [PATCH 32/43] Docstring fixes in length module and examples in Quantity Add note to Quanity explaining why explicitly passing units is recommended Make Quantity accessible from package namespace --- archeryutils/__init__.py | 3 ++- archeryutils/handicaps/handicap_scheme.py | 2 +- archeryutils/length.py | 16 ++++++------- archeryutils/rounds.py | 4 ++-- archeryutils/targets.py | 29 ++++++++++++++++++++++- archeryutils/tests/test_length.py | 4 +--- 6 files changed, 42 insertions(+), 16 deletions(-) diff --git a/archeryutils/__init__.py b/archeryutils/__init__.py index a087799..4addec5 100644 --- a/archeryutils/__init__.py +++ b/archeryutils/__init__.py @@ -2,7 +2,7 @@ from archeryutils import classifications, handicaps from archeryutils.rounds import Pass, Round -from archeryutils.targets import Target +from archeryutils.targets import Quantity, Target from archeryutils.utils import versions __all__ = [ @@ -10,6 +10,7 @@ "handicaps", "Pass", "Round", + "Quantity", "Target", "versions", ] diff --git a/archeryutils/handicaps/handicap_scheme.py b/archeryutils/handicaps/handicap_scheme.py index 019a0a9..9a58f2f 100644 --- a/archeryutils/handicaps/handicap_scheme.py +++ b/archeryutils/handicaps/handicap_scheme.py @@ -459,7 +459,7 @@ def handicap_from_score( To get an integer value as would appear in the handicap tables use ``int_prec=True``: - >>> agb_scheme.handicap_from_score(999, wa_outdoor.wa1440_90), int_prec=True) + >>> agb_scheme.handicap_from_score(999, wa_outdoor.wa1440_90, int_prec=True) 44.0 """ diff --git a/archeryutils/length.py b/archeryutils/length.py index 8c50e60..774096b 100644 --- a/archeryutils/length.py +++ b/archeryutils/length.py @@ -111,7 +111,7 @@ def to_metres(value: float, unit: str) -> float: Examples -------- - >>> convert.to_metres(10, "inches") + >>> length.to_metres(10, "inches") 0.254 """ return _conversions[unit] * value @@ -135,7 +135,7 @@ def from_metres(metre_value: float, unit: str) -> float: Examples -------- - >>> convert.from_metres(18.3, "yards") + >>> length.from_metres(18.3, "yards") 20.0131 """ return metre_value / _conversions[unit] @@ -157,7 +157,7 @@ def definitive_unit(alias: str) -> str: Examples -------- - >>> convert.definitive_unit("Metres") + >>> length.definitive_unit("Metres") "metre" """ return _reversed[alias] @@ -179,7 +179,7 @@ def definitive_units(aliases: Collection[str]) -> set[str]: Examples -------- - >>> convert.definitive_units(convert.metre | convert.yard) + >>> length.definitive_units(length.metre | length.yard) {'metre', 'yard'} """ return {_reversed[alias] for alias in aliases} @@ -227,12 +227,12 @@ def parse_optional_units( Examples -------- - >>> m_and_yd = convert.metre | convert.yard - >>> convert.parse_optional_units(10, m_and_yd, "metre") + >>> m_and_yd = length.metre | length.yard + >>> length.parse_optional_units(10, m_and_yd, "metre") (10, 'metre') - >>> convert.parse_optional_units((10, 'yards') m_and_yd, 'metre') + >>> length.parse_optional_units((10, "yards"), m_and_yd, "metre") (10, 'yard') - >>> convert.parse_optional_units((10, 'banana') m_and_yd, 'metre') + >>> length.parse_optional_units((10, "banana"), m_and_yd, "metre") ValueError: Unit 'banana' not recognised. Select from {'yard', 'metre'}. """ if default not in supported: diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 6dc95ec..e93ead9 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -88,8 +88,8 @@ def at_target( # noqa: PLR0913 explicitly specified using tuples: >>> myWA18pass = au.Pass.at_target( - 30, "10_zone", (40, "cm"), (18.0, "m"), indoor=True - ) + ... 30, "10_zone", (40, "cm"), (18.0, "m"), indoor=True + ... ) """ target = Target(scoring_system, diameter, distance, indoor) return cls(n_arrows, target) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index a4bb939..42efe9e 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -36,12 +36,39 @@ class Quantity(NamedTuple): Can be used in place of a plain tuple of (value, units) - Attributes + Parameters ---------- value: float Scalar value of quantity units: str Units of quantity + + Notes + ----- + It is recommened to use the `Quantity` type when passing specifyinging lengths + in _archeryutils_ for explictness and readability, and to ensure the expected units + are indeed being used downstream. Default units are assumed for convinience in + interactive use but this could cause breakages if the default unit changes + in the future. + + Examples + -------- + Define as a simple tuple: + + >>> worcester_distance = au.Quantity(80, "yard") + + Or with named keyword arguments: + + >>> worcester_target_size = au.Quantity(value=122, units="cm") + + These can then be passed on to any function that accepts a Quantity like tuple: + + >>> worcester_target = au.Target( + ... "Worcester", + ... diameter=worcester_target_size, + ... distance=worcester_distance, + ... indoor=True, + ... ) """ value: float diff --git a/archeryutils/tests/test_length.py b/archeryutils/tests/test_length.py index 26499ed..8079af1 100644 --- a/archeryutils/tests/test_length.py +++ b/archeryutils/tests/test_length.py @@ -117,6 +117,4 @@ def test_optional_unit_parsing_default_not_supported(self): with pytest.raises( ValueError, match="Default unit (.+) must be in supported units" ): - assert length.parse_optional_units( - 10, length.metre | length.yard, "inch" - ) + assert length.parse_optional_units(10, length.metre | length.yard, "inch") From f3e52428b4017ec59ee357c06ee509197718e44f Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 11 Mar 2024 13:46:12 +0000 Subject: [PATCH 33/43] Add default value of metre for default parameter in parse_optional_units --- archeryutils/length.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/archeryutils/length.py b/archeryutils/length.py index 774096b..b1c192d 100644 --- a/archeryutils/length.py +++ b/archeryutils/length.py @@ -188,7 +188,7 @@ def definitive_units(aliases: Collection[str]) -> set[str]: def parse_optional_units( value: Union[T, tuple[T, str]], supported: Set[str], - default: str, + default: str = "metre", ) -> tuple[T, str]: """ Parse single value or tuple of value and units. From 9de8048edbdd2128040e1b810978e1d6e1ac040d Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 11 Mar 2024 13:51:20 +0000 Subject: [PATCH 34/43] Simplifiy signature and return type of Round.max_distance() Now always returns a Quantity so no reason for users not to know what units the return value is and avoids potential footguns. Existing library code adjusted to extract the raw value without futher changes --- .../agb_outdoor_classifications.py | 6 +++--- archeryutils/rounds.py | 17 ++++------------- archeryutils/tests/test_rounds.py | 9 ++++++--- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/archeryutils/classifications/agb_outdoor_classifications.py b/archeryutils/classifications/agb_outdoor_classifications.py index 4fa46b0..cdcc966 100644 --- a/archeryutils/classifications/agb_outdoor_classifications.py +++ b/archeryutils/classifications/agb_outdoor_classifications.py @@ -325,7 +325,7 @@ def _assign_outdoor_prestige( # Check all other rounds based on distance for roundname in distance_check: - if ALL_OUTDOOR_ROUNDS[roundname].max_distance() >= np.min(max_dist): + if ALL_OUTDOOR_ROUNDS[roundname].max_distance().value >= np.min(max_dist): prestige_rounds.append(roundname) return prestige_rounds @@ -469,7 +469,7 @@ def _check_prestige_distance( # If not prestige, what classes are ineligible based on distance to_del: list[str] = [] - round_max_dist = ALL_OUTDOOR_ROUNDS[roundname].max_distance() + round_max_dist = ALL_OUTDOOR_ROUNDS[roundname].max_distance().value for class_i_name, class_i_data in class_data.items(): if class_i_data["min_dist"] > round_max_dist: to_del.append(class_i_name) @@ -557,7 +557,7 @@ def agb_outdoor_classification_scores( class_scores[0:3] = [-9999] * 3 # If not prestige, what classes are eligible based on category and distance - round_max_dist = ALL_OUTDOOR_ROUNDS[roundname].max_distance() + round_max_dist = ALL_OUTDOOR_ROUNDS[roundname].max_distance().value for i in range(3, len(class_scores)): if group_data["min_dists"][i] > round_max_dist: class_scores[i] = -9999 diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index e93ead9..8a9ca3a 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -235,21 +235,14 @@ def max_score(self) -> float: """ return sum(pass_i.max_score() for pass_i in self.passes) - def max_distance(self, unit: bool = False) -> Union[float, Quantity]: + def max_distance(self) -> Quantity: """ Return the maximum distance shot on this round along with the unit (optional). - Parameters - ---------- - unit : bool - Return unit as well as numerical value? - Returns ------- - max_dist : float - maximum distance shot in this round - (max_dist, unit) : tuple (float, str) - tuple of max_dist and string of unit + max_dist : Quantity + maximum distance and units shot in this round Notes ----- @@ -258,9 +251,7 @@ def max_distance(self, unit: bool = False) -> Union[float, Quantity]: whatever units it was defined in. """ longest_pass = max(self.passes, key=lambda p: p.distance) - if unit: - return longest_pass.native_distance - return longest_pass.native_distance.value + return longest_pass.native_distance def get_info(self) -> None: """ diff --git a/archeryutils/tests/test_rounds.py b/archeryutils/tests/test_rounds.py index 9652799..625383f 100644 --- a/archeryutils/tests/test_rounds.py +++ b/archeryutils/tests/test_rounds.py @@ -285,7 +285,10 @@ def test_max_distance( Pass.at_target(10, "5_zone", 122, (60, unit), False), ], ) - assert test_round.max_distance(unit=get_unit) == max_dist_expected + result = ( + test_round.max_distance() if get_unit else test_round.max_distance().value + ) + assert result == max_dist_expected def test_max_distance_out_of_order(self) -> None: """Check max distance correct when Passes not in descending distance order.""" @@ -297,7 +300,7 @@ def test_max_distance_out_of_order(self) -> None: Pass.at_target(10, "5_zone", 122, 60, False), ], ) - assert test_round.max_distance() == 100 + assert test_round.max_distance().value == 100 def test_max_distance_mixed_units(self) -> None: """Check that max distance accounts for different units in round.""" @@ -306,7 +309,7 @@ def test_max_distance_mixed_units(self) -> None: test_round = Round("test", [pyards, pmetric]) assert pmetric.distance > pyards.distance - assert test_round.max_distance() == 75 + assert test_round.max_distance().value == 75 def test_get_info(self, capsys: pytest.CaptureFixture[str]) -> None: """Check printing info works as expected.""" From 9f8fcb3a7d096188e2f30628ffbded784f19e06e Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 14:14:45 +0000 Subject: [PATCH 35/43] Document recommened use of Quantity for parameters --- docs/getting-started/quickstart.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/getting-started/quickstart.rst b/docs/getting-started/quickstart.rst index f745f69..67c845c 100644 --- a/docs/getting-started/quickstart.rst +++ b/docs/getting-started/quickstart.rst @@ -51,14 +51,16 @@ It can be invoked by specifying a scoring system, face size (cm) and distance (m mycompound720target = au.Target("10_zone_6_ring", 80, 50.0) In more complicated cases specific units can be passed in with the diameter and distance -as a tuple: +as a plain tuple, or (recommeneded for clarity) as a :py:class:`~archeryutils.targets.Quantity`: .. ipython:: python myWorcesterTarget = au.Target( "Worcester", diameter=(16, "inches"), distance=(20.0, "yards"), indoor=True ) - myIFAATarget = au.Target("IFAA_field", diameter=80, distance=(80.0, "yards")) + myIFAATarget = au.Target( + "IFAA_field", diameter=80, distance=au.Quantity(80.0, "yards") + ) If the target you want is not supported, you can manually supply the target ring sizes and scores as a `FaceSpec` and construct a target as so. From eca1bc67900dc43a7d327196dfd3416aab1c902e Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 13:59:52 +0000 Subject: [PATCH 36/43] Document supported units for distance and diameter under private class attributes --- archeryutils/targets.py | 5 +++++ docs/api/archeryutils.baseclasses.rst | 6 ++++++ docs/getting-started/quickstart.rst | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 42efe9e..d618f3d 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -130,8 +130,13 @@ class Target: _face_spec: FaceSpec + #: Allowable scoring systems that this target can utilise. _supported_systems = get_args(ScoringSystem) + + #: Allowable units and alises for target distances. _supported_distance_units = length.yard | length.metre + + #: Allowable units and alises for target diameters. _supported_diameter_units = length.cm | length.inch | length.metre def __init__( diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index 0b6b0ff..c100323 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -7,6 +7,12 @@ Base Classes :undoc-members: :show-inheritance: +The following units and aliases are recognised as valid for specifiying target diameters and distances: + +.. autoattribute:: archeryutils.Target._supported_diameter_units + +.. autoattribute:: archeryutils.Target._supported_distance_units + .. autoclass:: archeryutils.Pass :members: :undoc-members: diff --git a/docs/getting-started/quickstart.rst b/docs/getting-started/quickstart.rst index 67c845c..4f78726 100644 --- a/docs/getting-started/quickstart.rst +++ b/docs/getting-started/quickstart.rst @@ -73,7 +73,7 @@ and scores as a `FaceSpec` and construct a target as so. print(myKingsTarget.scoring_system) .. note:: - Although we can provide the face_spec in any unit listed under :py:attr:`~archeryutils.Target.supported_diameter_units`, + Although we can provide the face_spec in any unit listed under :py:attr:`~archeryutils.Target._supported_diameter_units`, the sizes in the specification are converted to metres and stored in this form. Therefore unlike the target diameter paramater, the default unit for specifications is [metres] From d252ce276e4fb05bc9b24d930940cd6168deff63 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 14:02:20 +0000 Subject: [PATCH 37/43] Use napoleon to process types explicitly Allows deduplication of explicitly listing supported scoring systems inside the Target docstring and instead refers to the existing ScoringSystem type alias. --- archeryutils/targets.py | 8 ++------ docs/api/archeryutils.baseclasses.rst | 8 ++++++++ docs/conf.py | 5 +++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index d618f3d..a7d05b9 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -81,11 +81,7 @@ class Target: Parameters ---------- - scoring_system : {\ - ``"5_zone"`` ``"10_zone"`` ``"10_zone_compound"`` ``"10_zone_6_ring"``\ - ``"10_zone_5_ring"`` ``"10_zone_5_ring_compound"`` ``"WA_field"``\ - ``"IFAA_field"`` ``"IFAA_field_expert"`` ``"Beiter_hit_miss"`` ``"Worcester"``\ - ``"Worcester_2_ring"``} + scoring_system : ScoringSystem target face/scoring system type. Must be one of the supported values. diameter : float or tuple of float, str Target face diameter default [centimetres]. @@ -354,7 +350,7 @@ def gen_face_spec(system: ScoringSystem, diameter: float) -> FaceSpec: Returns ------- - spec : dict + spec : FaceSpec Mapping of target ring sizes in [metres] to score Raises diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index c100323..8a73456 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -23,3 +23,11 @@ The following units and aliases are recognised as valid for specifiying target d :undoc-members: :show-inheritance: +Types +----- + +.. autodata:: archeryutils.targets.ScoringSystem + :annotation: bob + +End of file + diff --git a/docs/conf.py b/docs/conf.py index 1c3dec5..244b1b4 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -46,3 +46,8 @@ html_theme = "sphinx_rtd_theme" html_static_path = ["_static"] autodoc_member_order = "bysource" +napoleon_preprocess_types = True +napoleon_type_aliases = { + "FaceSpec": "~archeryutils.targets.FaceSpec", + "ScoringSystem": "~archeryutils.targets.ScoringSystem", +} From ff055c426a5825e9689ba17805b3d08105cdf169 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 14:46:09 +0000 Subject: [PATCH 38/43] Fix end of baseclases documentation, inline documentation comments for type aliases, create links for FaceSpec --- archeryutils/rounds.py | 6 +----- archeryutils/targets.py | 3 +++ docs/api/archeryutils.baseclasses.rst | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 8a9ca3a..127f83e 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -62,11 +62,7 @@ def at_target( # noqa: PLR0913 ---------- n_arrows : int number of arrows in this pass - scoring_system : {\ - ``"5_zone"`` ``"10_zone"`` ``"10_zone_compound"`` ``"10_zone_6_ring"``\ - ``"10_zone_5_ring"`` ``"10_zone_5_ring_compound"`` ``"WA_field"``\ - ``"IFAA_field"`` ``"IFAA_field_expert"`` ``"Beiter_hit_miss"`` ``"Worcester"``\ - ``"Worcester_2_ring"``} + scoring_system : ScoringSystem target face/scoring system type diameter : float or tuple of float, str Target diameter size (and units, default [cm]) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index a7d05b9..7205f4c 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -8,6 +8,7 @@ from archeryutils import length # TypeAlias (annotate explicitly in py3.10+) +#: All scoring systems that archeryutils knows how to handle by default. ScoringSystem = Literal[ "5_zone", "10_zone", @@ -25,6 +26,8 @@ ] # TypeAlias (annotate explicitly in py3.10+) +#: A mapping of a target ring diameter to the score for hitting that ring. +#: Typically ordered from smallest to largest rings but this is not necessary. FaceSpec = Mapping[float, int] _rnd6 = partial(round, ndigits=6) diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index 8a73456..0eea1a1 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -27,7 +27,5 @@ Types ----- .. autodata:: archeryutils.targets.ScoringSystem - :annotation: bob - -End of file +.. autodata:: archeryutils.targets.FaceSpec From e0239a6a0aebd8f04b9b740f4fd364c39b06850d Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 14:51:20 +0000 Subject: [PATCH 39/43] Repeat limitations on face_spec described in quickstart into Target.from_face_spec docstring --- archeryutils/targets.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 7205f4c..a12141d 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -204,6 +204,15 @@ def from_face_spec( Instance of Target class with scoring system set as "Custom" and face specification stored. + Notes + ----- + Targets created in this way can represent almost any common round target in use, + altough archeryutils has built in support for many of the most popular. + There are some limitations to the target faces that can be represented however: + + 1. Targets must be formed of concentric rings + 2. The score must monotonically decrease as the rings get larger + Examples -------- >>> # Kings of archery recurve scoring triple spot From 995681fa464b243ebfa9f7cc8ec17cf60d14c644 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Tue, 12 Mar 2024 23:05:09 +0000 Subject: [PATCH 40/43] Seperate type aliases to their own page in API docs --- docs/api/archeryutils.baseclasses.rst | 6 ------ docs/api/archeryutils.types.rst | 8 ++++++++ docs/api/index.rst | 5 +++++ 3 files changed, 13 insertions(+), 6 deletions(-) create mode 100644 docs/api/archeryutils.types.rst diff --git a/docs/api/archeryutils.baseclasses.rst b/docs/api/archeryutils.baseclasses.rst index 0eea1a1..c100323 100644 --- a/docs/api/archeryutils.baseclasses.rst +++ b/docs/api/archeryutils.baseclasses.rst @@ -23,9 +23,3 @@ The following units and aliases are recognised as valid for specifiying target d :undoc-members: :show-inheritance: -Types ------ - -.. autodata:: archeryutils.targets.ScoringSystem - -.. autodata:: archeryutils.targets.FaceSpec diff --git a/docs/api/archeryutils.types.rst b/docs/api/archeryutils.types.rst new file mode 100644 index 0000000..427d7e9 --- /dev/null +++ b/docs/api/archeryutils.types.rst @@ -0,0 +1,8 @@ +Types +===== + +.. autodata:: archeryutils.targets.ScoringSystem + +.. autodata:: archeryutils.targets.FaceSpec + + diff --git a/docs/api/index.rst b/docs/api/index.rst index a146f29..44e3dac 100644 --- a/docs/api/index.rst +++ b/docs/api/index.rst @@ -8,6 +8,11 @@ API Documentation archeryutils.baseclasses +.. toctree:: + :maxdepth: 3 + + archeryutils.types + .. toctree:: :maxdepth: 3 From 269b8a2f377f4a02b1caad0ad3ff3458b8912d36 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Wed, 13 Mar 2024 00:05:38 +0000 Subject: [PATCH 41/43] Update property docs for passthrough attributes on Pass --- archeryutils/rounds.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 127f83e..86697ac 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -107,7 +107,7 @@ def scoring_system(self) -> ScoringSystem: @property def diameter(self) -> float: - """Get target diameter [metres].""" + """Get target diameter in [metres].""" return self.target.diameter @property @@ -117,12 +117,12 @@ def distance(self) -> float: @property def native_diameter(self) -> Quantity: - """Get native_diameter_unit attribute of target.""" + """Get diameter of target in native units.""" return self.target.native_diameter @property def native_distance(self) -> Quantity: - """Get native_dist_unit attribute of target.""" + """Get distance of target in native units.""" return self.target.native_distance @property @@ -216,9 +216,9 @@ def __eq__(self, other: object) -> bool: Does not consider optional labels of location/body/family as these do not affect behaviour. """ - if isinstance(other, Round): - return self.name == other.name and self.passes == other.passes - return NotImplemented + if not isinstance(other, Round): + return NotImplemented + return self.name == other.name and self.passes == other.passes def max_score(self) -> float: """ From a4bd13281cb0265e3a2ebaf05d45549a8b280be4 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 18 Mar 2024 07:10:20 +0000 Subject: [PATCH 42/43] Rearrange types in docs, touch up FaceSpec doc-comment and link from to target constructor for more details --- archeryutils/targets.py | 3 +-- docs/api/archeryutils.types.rst | 2 +- docs/api/index.rst | 8 ++++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/archeryutils/targets.py b/archeryutils/targets.py index a12141d..57ae3aa 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -26,8 +26,7 @@ ] # TypeAlias (annotate explicitly in py3.10+) -#: A mapping of a target ring diameter to the score for hitting that ring. -#: Typically ordered from smallest to largest rings but this is not necessary. +#: A mapping of a target ring diameter to the score for that ring. FaceSpec = Mapping[float, int] _rnd6 = partial(round, ndigits=6) diff --git a/docs/api/archeryutils.types.rst b/docs/api/archeryutils.types.rst index 427d7e9..d9a316c 100644 --- a/docs/api/archeryutils.types.rst +++ b/docs/api/archeryutils.types.rst @@ -5,4 +5,4 @@ Types .. autodata:: archeryutils.targets.FaceSpec - +See :py:meth:`~archeryutils.Target.from_face_spec` for more details what targets can be modeled with this. diff --git a/docs/api/index.rst b/docs/api/index.rst index 44e3dac..8bb47c4 100644 --- a/docs/api/index.rst +++ b/docs/api/index.rst @@ -4,14 +4,14 @@ API Documentation ================= .. toctree:: - :maxdepth: 2 + :maxdepth: 3 - archeryutils.baseclasses + archeryutils.types .. toctree:: - :maxdepth: 3 + :maxdepth: 2 - archeryutils.types + archeryutils.baseclasses .. toctree:: :maxdepth: 3 From 879e9bc814578fcc0996dd9438bfab19cf1aafb8 Mon Sep 17 00:00:00 2001 From: Tom Hall Date: Mon, 18 Mar 2024 07:14:30 +0000 Subject: [PATCH 43/43] Clarify in docstrings that ScoringSystem is a literal string value --- archeryutils/rounds.py | 2 +- archeryutils/targets.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/archeryutils/rounds.py b/archeryutils/rounds.py index 86697ac..1a1a7ca 100644 --- a/archeryutils/rounds.py +++ b/archeryutils/rounds.py @@ -63,7 +63,7 @@ def at_target( # noqa: PLR0913 n_arrows : int number of arrows in this pass scoring_system : ScoringSystem - target face/scoring system type + Literal string value of target face/scoring system type. diameter : float or tuple of float, str Target diameter size (and units, default [cm]) distance : float or tuple of float, str diff --git a/archeryutils/targets.py b/archeryutils/targets.py index 57ae3aa..a538689 100644 --- a/archeryutils/targets.py +++ b/archeryutils/targets.py @@ -84,7 +84,8 @@ class Target: Parameters ---------- scoring_system : ScoringSystem - target face/scoring system type. Must be one of the supported values. + Literal string value of target face/scoring system type. + Must be one of the supported values. diameter : float or tuple of float, str Target face diameter default [centimetres]. distance : float or tuple of float, str