scrazzz / redgifs

Simple Python API wrapper for the RedGIFs API
https://redgifs.rtfd.io
MIT License
91 stars 12 forks source link

The problem with the random method #5

Closed AaronHuston closed 2 years ago

AaronHuston commented 2 years ago

Hello! Faced some problems with the random method. The following error appears:

'local variable 'st' referenced before assignment'

api = redgifs.API() result = api.search( Tags.random(count=1), order=Order.best,
count=72, page=input()
)

scrazzz commented 2 years ago

The issue is that Tags.random(1) gives a list. Since search() method doesn't recognise a list, it raises this error.

Doing this might help:

random = Tags.random(1) # ['xyz']
result = api.search(random[0]) # random[0] now gives 'xyz' (string)

I'll make it handle this issue properly in the next version release so it may take some time.

anytarseir67 commented 2 years ago

i'd like to point out that this is not really a bug, this should be the expected behavior. if the "normal" usage of the function results in a list, there is no reason that you should change that for edge cases.

scrazzz commented 2 years ago

i'd like to point out that this is not really a bug, this should be the expected behavior. if the "normal" usage of the function results in a list, there is no reason that you should change that for edge cases.

Yes, you're right. I have thought of the same thing too. But then I have planned to return just the string if random(n=1) and return a list of string if n > 1.

def random(n: int = 1) Union[str, List[str]:
    if n == 1:
        # do fuzzy search here
        return just_string

    # normal functioning of method here

This would be easier and clean because sometimes you just want a quick way of getting the result without doing random(1)[0] everytime.

Now that you have mentioned it, I'm not sure wheather to keep the random function as-is or to change it.

anytarseir67 commented 2 years ago

Yes, you're right. I have thought of the same thing too. But then I have planned to return just the string if random(n=1) and return a list of string if n > 1.

def random(n: int = 1) Union[str, List[str]:
    if n == 1:
        # do fuzzy search here
        return just_string

    # normal functioning of method here

This would be easier and clean because sometimes you just want a quick way of getting the result without doing random(1)[0] everytime.

Now that you have mentioned it, I'm not sure wheather to keep the random function as-is or to change it.

i think you missed the point of what i was trying to say, unless there is some need to, a function should always return the same type.

for example, if i were to take a number from the user, and get that many tags, i would have to handle it differently depending on what number they pass

def get_tags(num: int) -> List[str]:
    return Tags.random(num)

# vs

def get_tags(num: int) -> List[str]:
    x = Tags.random(num)
    if num == 1:
        x = [x]
    return x

if you really want to handle in-lib, you should probably add another method, that only gets 1 tag

scrazzz commented 2 years ago

if you really want to handle in-lib, you should probably add another method, that only gets 1 tag

Yes, you're right. Though I would not know what I would call that method as.

I have planned to not change this method, it'll be as-is.

To clarify @AaronHuston, you should be indexing Tags.random() method as mentioned here: https://github.com/scrazzz/redgifs/issues/5#issuecomment-1219638123