Repository: pallets/click
Pull request #2829 (2024-12-22 13:39:37)

Only try to set flag_value if is_flag is true
This statement:

    flag_value = not self.default

requires the class to have a working `__bool__` method. However, there are some classes that explicitly disable this method:

- [`numpy.ndarray`](https://numpy.org/doc/stable/reference/arrays.ndarray.html) raises `ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()`
- [`astropy.units.Quantity`](https://docs.astropy.org/en/stable/api/astropy.units.Quantity.html) raises `ValueError: Quantity truthiness is ambiguous, especially for logarithmic units and temperatures. Use explicit  comparisons.`

Move this statement so that it is only executed if `is_flag` is true.

<!--
Before opening a PR, open a ticket describing the issue or feature the
PR will address. An issue is not required for fixing typos in
documentation, or other simple non-code changes.

Replace this comment with a description of the change. Describe how it
addresses the linked ticket.
-->

<!--
Link to relevant issues or previous PRs, one per line. Use "fixes" to
automatically close an issue.

fixes #<issue number>
-->

<!--
Ensure each step in CONTRIBUTING.rst is complete, especially the following:

- Add tests that demonstrate the correct behavior of the change. Tests
  should fail without the change.
- Add or update relevant docs, in the docs folder and in code.
- Add an entry in CHANGES.rst summarizing the change and linking to the issue.
- Add `.. versionchanged::` entries in any relevant code docs.
-->


lpsinger: See, for example, https://github.com/m4opt/m4opt/issues/220.
AndreasBackx: This seems to make sense. Thank you! Here it would make sense to have some typing to clarify this requirement. Though that's out of scope for this PR.

Patch:
diff --git a/CHANGES.rst b/CHANGES.rst
index 1a38be8..bebfef2 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -91,6 +91,9 @@ Unreleased
 -   Add a ``catch_exceptions`` parameter to :class:`CliRunner`. If
     ``catch_exceptions`` is not passed to :meth:`CliRunner.invoke`,
     the value from :class:`CliRunner`. :issue:`2817` :pr:`2818`
+-   ``Option.flag_value`` will no longer have a default value set based on
+    ``Option.default`` if ``Option.is_flag`` is ``False``. This results in
+    ``Option.default`` not needing to implement `__bool__`. :pr:`2829`
 
 Version 8.1.8
 -------------
diff --git a/src/click/core.py b/src/click/core.py
index 1c1a467..e783729 100644
--- a/src/click/core.py
+++ b/src/click/core.py
@@ -2597,11 +2597,10 @@ class Option(Parameter):
             else:
                 self.default = False
 
-        if flag_value is None:
-            flag_value = not self.default
-
         self.type: types.ParamType
         if is_flag and type is None:
+            if flag_value is None:
+                flag_value = not self.default
             # Re-guess the type from the flag value instead of the
             # default.
             self.type = types.convert_type(None, flag_value)


Test patch:
diff --git a/tests/test_info_dict.py b/tests/test_info_dict.py
index 79d39ee..11b6703 100644
--- a/tests/test_info_dict.py
+++ b/tests/test_info_dict.py
@@ -58,7 +58,7 @@ NUMBER_OPTION = (
         "help": None,
         "prompt": None,
         "is_flag": False,
-        "flag_value": False,
+        "flag_value": None,
         "count": False,
         "hidden": False,
     },