Unbound local x in percentage_to_str

[Poikilos]

There is a question of what your intention was here. Reading the code seems to indicate that you meant to have this missing line:

def percentage_to_str(v: int | float, **kwargs: dict) -> str:
+    x = v

Did you plan to force an exception here if the type is wrong?

If so, this would be more clear:

                return f"{int(round(x, kwargs['decimals']))}%"
+    else:
+        raise TypeError(f"Expected int or float, but got {type(v).__name__}({repr(v)})")
    return f"{x}%"

I wasn't 100% sure otherwise I'd do a pull request (still can if you clarify).

[ragardner]

Hello,

Thanks for bringing this issue to my attention

I had a look through the chain of functions and decided that there's not a guarantee that v will be an int or float, so I made a slight change:

def percentage_to_str(v: object, **kwargs: dict) -> str:
    if isinstance(v, (int, float)):
        x = v * 100
        if isinstance(x, float):
            if x.is_integer():
                return f"{int(x)}%"
            if "decimals" in kwargs and isinstance(kwargs["decimals"], int):
                if kwargs["decimals"]:
                    return f"{round(x, kwargs['decimals'])}%"
                return f"{int(round(x, kwargs['decimals']))}%"
        return f"{x}%"
    return f"{v}%"

[Poikilos]

lgtm. int will always be 0 or 100 (if math before calling this is correct) but I guess that's the only way to be consistent.

[ragardner]

By default the percentage_formatter() will make v a float. A developer would have to make their own version of to_float in order to force the v parameter receive an int. e.g.

def my_new_func(s, **kws):
    if s.endswith("%"):
        return int(float(s.replace("%", "")) / 100)
    return int(float(s))

sheet.format(
    "A",
    formatter_options=percentage_formatter(
        format_function=my_new_func,
        nullable=False,
    ),
)

I have left it open to the possibility of the function receiving an int just in case, although I can't think why it might be the case

There may also be the possibility of formatting using their own data types although you would assume if that were the case they would also provide their own to string function

Finally, it's intentional that the default percentage_formatter() converts user entry like so:

# 0.5   -> 50%
# 0.5%  -> 0.5%
# 1     -> 100%
# 1%    -> 1%
# 2000  -> 200000%
# 2000% -> 2000%

I will make an alternative to the default keyword arguments which would make it as if the user had put a % character at the end of the input