From 37a871eab1b58afc7abd10d40dd1f4a917670b3c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=A9mie=20Bolduc?=
 <16137441+Jouramie@users.noreply.github.com>
Date: Sun, 3 Mar 2024 16:30:51 -0500
Subject: [PATCH] Core: Allow common collections in OptionSet and OptionList
 constructors (#2874)

* allow common collection in set and list option constructors

* allow any iterable of strings

* add return None

---------

Co-authored-by: beauxq <beauxq@yahoo.com>
---
 Options.py       | 25 +++++++++++++------------
 Utils.py         | 11 +++++++++++
 requirements.txt |  1 +
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/Options.py b/Options.py
index 5fe4087132f7..139dc0a0bbe6 100644
--- a/Options.py
+++ b/Options.py
@@ -1,19 +1,18 @@
 from __future__ import annotations
 
 import abc
-import logging
-from copy import deepcopy
-from dataclasses import dataclass
 import functools
+import logging
 import math
 import numbers
 import random
 import typing
 from copy import deepcopy
+from dataclasses import dataclass
 
 from schema import And, Optional, Or, Schema
 
-from Utils import get_fuzzy_results
+from Utils import get_fuzzy_results, is_iterable_of_str
 
 if typing.TYPE_CHECKING:
     from BaseClasses import PlandoOptions
@@ -59,6 +58,7 @@ def __new__(mcs, name, bases, attrs):
                 def verify(self, *args, **kwargs) -> None:
                     for f in verifiers:
                         f(self, *args, **kwargs)
+
                 attrs["verify"] = verify
             else:
                 assert verifiers, "class Option is supposed to implement def verify"
@@ -183,6 +183,7 @@ def get_option_name(cls, value: str) -> str:
 
 class NumericOption(Option[int], numbers.Integral, abc.ABC):
     default = 0
+
     # note: some of the `typing.Any`` here is a result of unresolved issue in python standards
     # `int` is not a `numbers.Integral` according to the official typestubs
     # (even though isinstance(5, numbers.Integral) == True)
@@ -598,7 +599,7 @@ def verify(self, world: typing.Type[World], player_name: str, plando_options: "P
         if isinstance(self.value, int):
             return
         from BaseClasses import PlandoOptions
-        if not(PlandoOptions.bosses & plando_options):
+        if not (PlandoOptions.bosses & plando_options):
             # plando is disabled but plando options were given so pull the option and change it to an int
             option = self.value.split(";")[-1]
             self.value = self.options[option]
@@ -765,7 +766,7 @@ class VerifyKeys(metaclass=FreezeValidKeys):
     value: typing.Any
 
     @classmethod
-    def verify_keys(cls, data: typing.List[str]):
+    def verify_keys(cls, data: typing.Iterable[str]) -> None:
         if cls.valid_keys:
             data = set(data)
             dataset = set(word.casefold() for word in data) if cls.valid_keys_casefold else set(data)
@@ -843,11 +844,11 @@ class OptionList(Option[typing.List[typing.Any]], VerifyKeys):
     # If only unique entries are needed and input order of elements does not matter, OptionSet should be used instead.
     # Not a docstring so it doesn't get grabbed by the options system.
 
-    default: typing.List[typing.Any] = []
+    default: typing.Union[typing.List[typing.Any], typing.Tuple[typing.Any, ...]] = ()
     supports_weighting = False
 
-    def __init__(self, value: typing.List[typing.Any]):
-        self.value = deepcopy(value)
+    def __init__(self, value: typing.Iterable[str]):
+        self.value = list(deepcopy(value))
         super(OptionList, self).__init__()
 
     @classmethod
@@ -856,7 +857,7 @@ def from_text(cls, text: str):
 
     @classmethod
     def from_any(cls, data: typing.Any):
-        if type(data) == list:
+        if is_iterable_of_str(data):
             cls.verify_keys(data)
             return cls(data)
         return cls.from_text(str(data))
@@ -882,7 +883,7 @@ def from_text(cls, text: str):
 
     @classmethod
     def from_any(cls, data: typing.Any):
-        if isinstance(data, (list, set, frozenset)):
+        if is_iterable_of_str(data):
             cls.verify_keys(data)
             return cls(data)
         return cls.from_text(str(data))
@@ -932,7 +933,7 @@ def __new__(mcs,
                 bases: typing.Tuple[type, ...],
                 attrs: typing.Dict[str, typing.Any]) -> "OptionsMetaProperty":
         for attr_type in attrs.values():
-            assert not isinstance(attr_type, AssembleOptions),\
+            assert not isinstance(attr_type, AssembleOptions), \
                 f"Options for {name} should be type hinted on the class, not assigned"
         return super().__new__(mcs, name, bases, attrs)
 
diff --git a/Utils.py b/Utils.py
index da2d837ad3a3..cea6405a38b4 100644
--- a/Utils.py
+++ b/Utils.py
@@ -19,6 +19,7 @@
 from argparse import Namespace
 from settings import Settings, get_settings
 from typing import BinaryIO, Coroutine, Optional, Set, Dict, Any, Union
+from typing_extensions import TypeGuard
 from yaml import load, load_all, dump
 
 try:
@@ -966,3 +967,13 @@ def __bool__(self):
 
     def __len__(self):
         return sum(len(iterable) for iterable in self.iterable)
+
+
+def is_iterable_of_str(obj: object) -> TypeGuard[typing.Iterable[str]]:
+    """ but not a `str` (because technically, `str` is `Iterable[str]`) """
+    if isinstance(obj, str):
+        return False
+    if not isinstance(obj, typing.Iterable):
+        return False
+    obj_it: typing.Iterable[object] = obj
+    return all(isinstance(v, str) for v in obj_it)
diff --git a/requirements.txt b/requirements.txt
index e2ccb67c18d4..9531e3058e8a 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -11,3 +11,4 @@ certifi>=2023.11.17
 cython>=3.0.8
 cymem>=2.0.8
 orjson>=3.9.10
+typing-extensions>=4.7.0