OZtree icon indicating copy to clipboard operation
OZtree copied to clipboard

`otts2vns` API endpoint considers 0 to be true.

Open fwimp opened this issue 1 year ago • 4 comments

When hitting the ott to common name endpoint (https://www.onezoom.org/API/otts2vns), the query all=0 is the same as all=on or all=true.

More specifically I actually cannot find any way to provide this argument that does not switch it on.

Requests that count as "on"

https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en&all=false https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en&all=0 https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en&all=none https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en&all=off

It looks like this issue comes from https://github.com/OneZoom/OZtree/blob/889026e68377ae306e8f3ffc60e16223803420cc/controllers/API.py#L719 where the truthiness of a filled string is always checked rather than trying to cast said string first.

In the popularity endpoint it looks like there is a custom function that could be duplicated over to solve this issue: https://github.com/OneZoom/OZtree/blob/889026e68377ae306e8f3ffc60e16223803420cc/controllers/popularity.py#L296-L297

fwimp avatar Aug 01 '24 09:08 fwimp

Omitting it (https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en) or an empty string (https://www.onezoom.org/API/otts2vns?key=0&otts=247341&lang=en&all=) will turn it off, but you're right, this is a bit confusing.

There may be a helper in gluon/web2py to use here rather than rolling our own.

Thanks!

lentinj avatar Aug 01 '24 09:08 lentinj

Yeah that's my fix for the moment is to check whether it the argument is false and then not include it in the query if that is the case.

I actually believe that gluon does not have IS_BOOL or similar as a validator.

I remember rolling our own years ago for another project. This would not do the conversion directly, but leverages strtobool() from the distutils package, which could be a viable approach here too :)

fwimp avatar Aug 01 '24 10:08 fwimp

distutils.strtobool is getting deprecated sadly: https://github.com/drgarcia1986/simple-settings/issues/273

Rolling our own would also allow us to consider doing without the lower(), which would be nice if we could.

lentinj avatar Aug 01 '24 10:08 lentinj

Ahh that's frustrating.

Looks like the original code of strtobool was just this:

def strtobool(val):
    """Convert a string representation of truth to true (1) or false (0).

    True values are 'y', 'yes', 't', 'true', 'on', and '1'; false values
    are 'n', 'no', 'f', 'false', 'off', and '0'.  Raises ValueError if
    'val' is anything else.
    """
    val = val.lower()
    if val in ('y', 'yes', 't', 'true', 'on', '1'):
        return 1
    elif val in ('n', 'no', 'f', 'false', 'off', '0'):
        return 0
    else:
        raise ValueError(f"invalid truth value {val!r}")

Wouldn't get rid of lower(), though honestly given that bools are inconsistently capitalised between languages when cast to strings, this could actually be required. (e.g. R converts as FALSE while python converts as False)

fwimp avatar Aug 01 '24 10:08 fwimp