Should bool(Unit()) be False ?

[neutrinoceros]

• unyt version: 2.9.5
• Python version: any
• Operating System: any

Description

Given how most builtin types implement "truthiness" (where 0 or empty objects evaluate to False), I'd expect bool(Unit()) to evaluate to False, though currently every unit, even empty ones, evaluate to True.

Such a change might be considered breaking (though it's probably very niche ?), so I wonder if there's any objections to changing this for unyt 3.0 ?

What I Did

from unyt import Unit
assert not Unit()

[neutrinoceros]

Just to give a little more context, I think this would allow to write more idiomatic code in situations where we need to grab some unit from a pair of arrays that might be pure ndarrays:

NULL_UNIT = Unit()
units = getattr(a, "units", NULL_UNIT) or getattr(b, "units", NULL_UNIT)

(currently b.units would never be evaluated in this code, because NULL_UNIT is truthy)

The following patch is sufficient to allow this:

diff --git a/unyt/unit_object.py b/unyt/unit_object.py
index 2b13e46..e55e891 100644
--- a/unyt/unit_object.py
+++ b/unyt/unit_object.py
@@ -513,6 +513,8 @@ class Unit:
     def __deepcopy__(self, memodict=None):
         return self.copy(deep=True)
 
+    def __bool__(self):
+        return self != NULL_UNIT
     #
     # End unit operations
     #

and I checked that it doesn't break any existing test

[ngoldbaum]

Hmm, I'm not sure about this. Have you tried running the tests against yt with this change? On the one hand, since units have never been falsy, it's unlikely people have been using them in a situation where this change to sometimes being falsey would matter. On the other hand, it's a common enough thing to do and it might subtly impact the behavior of a complex expression. Unfortunately I don't think it's possible to deprecate this behavior so we can't shake it out in the wild either.

Just to be on the safe side, I would prefer to not do this. However if you can come up with a more compelling story than making some internal code in unyt a bit simpler than I might be persuaded.

Also wrt to the proposed implementation, I would also prefer to keep this logic in __ne__ and __eq__ if possible rather than adding a new dunder method implementation people might have to be aware of if they want to fully understand how unit equality works.

[neutrinoceros]

Have you tried running the tests against yt with this change?

just did. No error in sight.

Just to be on the safe side, I would prefer to not do this. However if you can come up with a more compelling story than making some internal code in unyt a bit simpler then I might be persuaded.

Honestly, I don't have much of a case here. Feel free to close.