1

Here's the code:

def url_is_image(url):
    if url[-4:] in ['.jpg', '.png']:
        return True
    elif 'imgur' in url:
        url += '.jpg'
        return True
    else:
        return False

u1 = 'http://i.imgur.com/jtboy'
url_is_image(u1)
print u1

Output is 'http://i.imgur.com/jtboy' without the .jpg extension. Any ideas for why my function (line 5) fails to add .jpg to the string? Thanks

Improve this question
4
  • 2
    Please, fix your indentation first! Commented Dec 23, 2015 at 16:14
  • 1
    You are only changing url inside url_is_image ... Commented Dec 23, 2015 at 16:16
  • 2
    I would replace if url[-4:] in ['.jpg', '.png']: with the much clearer if url.endswith(('.jpg', '.png')):. Commented Dec 23, 2015 at 16:28
  • 1
    Possible duplicate of Python immutable object from within function Commented Dec 23, 2015 at 16:32

5 Answers 5

Reset to default
5

The scope of the argument url is local to the call to the function. Assignment to it does not affect any variable or value outside the function.

The variable u1 was assigned to once. The function call does not affect it. Then it is printed.

Since you didn't test the truthiness of the function's return value, you could rewrite it this way:

def url_is_image(url):
    if url[-4:] in ['.jpg', '.png']:
        return url
    elif 'imgur' in url:
        return url + '.jpg'
    else:
        return url

u1 = 'http://i.imgur.com/jtboy'
u1 = url_is_image(u1)
print u1

However, the function name is misleading in this case. Since I don't know how you expect to really use the function it is hard to suggest a better solution. Maybe you want to return a 2-tuple containing the boolean and the modified string.

Addendum: based on the purpose you outlined in your comment here I would consider a function that returns only the interesting URLs. For example:

def consider_url(url):
    if url[-4:] in ['.jpg', '.png']:
        return url
    elif 'imgur' in url:
        return url + '.jpg'
    else:
        return None

Then you can use this to both transform the interesting URLs and ignore the uninteresting ones.

for url in some_list:
    url = consider_url(url)
    if url is None:
        continue
    print(url)

Another approach is to write a generator that provides you only the interesting urls:

def interesting_urls(iter):
    for url in iter:
        if url[-4:] in ['.jpg', '.png']:
            yield url
        elif 'imgur' in url:
            yield url + '.jpg'

Then you can process your urls even more succinctly:

for url in interesting_urls(some_list):
    print(url)

These examples assume that somehow you have already retrieved the URLs in a list or other iterable object. You can try it with a simple list such as the following. Although most of the strings in the list are not valid URLs, they do exercise the logic of the functions above.

some_list = [
    "foo", "bar.jpg", "something/imgur/baz", "uninteresting", 'http://i.imgur.com/jtboy'
]
Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

The function name implies a boolean return. But his usage of it below implies a string return: u1 = url_is_image(u1). Why would you want u1 to be set as a boolean?
@jphollowed: The OP mentioned something like this in a comment to another answer.
Thanks, I understand now. I've moved the fixing of the URL elsewhere so I'm not trying to test the URL and change it at the same time. The function is to be used on a list of links from a subreddit's main page. By passing every link on the page through this function I can separate links that are images (the ones I'm interested in) from other links. In the end I want a list of links that go straight to the image source, meaning they have to end in an image extension like .jpg. I changed this function so it's just a test and then I manipulate strings if necessary after using if statements.
@couragewolf Okay, that context helps. I like to avoid duplicate logic and linear searches (ie the "if "imgur" in url') when practical both for code maintenance and performance. I added to my answer two examples of designs that achieve this. Some of the other answers show how to return a two-tuple containing the boolean flag along with the modified string. I like the generator-based filter the best.
@dsh thanks very much for the addendum, it helps a lot!
1

try this:

def url_is_image(url):
    status = False
    ret = url
    if url[-4:] in ['.jpg', '.png']:
        status = True
    elif 'imgur' in url:
        ret = url + '.jpg'
        status = True

    return status, ret

u1 = 'http://i.imgur.com/jtboy'
stat, u1 = url_is_image(u1)  
print stat, u1
Improve this answer

4 Comments

Of course you can do a="one"; a+="two". a will be "onetwo".
value += '.jpg' is valid Python. When you use this syntax Python doesn't modify the string in-place but creates a new one and binds it to the old name.
true - i used a literal to test - my bad
I like this way as well, even if it is less explicit
0

You have to add the "global" keyword to your function. Just add a line "global url" to the very top of it. Also remove the parameter.

What you try to do is a "side-effect". The function modifies a variable outside of its scope. This can cause headaches to readers, especially when you name the function "url_is_image", where a reader would expect only a check and not a modification.

I'd suggest to move the side-effect to a separate function "fix_imgur_jpg_url", that takes the url as a parameter and returns the fixed url.

Improve this answer

Comments

0

Because when you pass url to url_is_image, it is creating a new url object. Modify your return statement. Don't use a global variable

def url_is_image(url):
    if url[-4:] in ['.jpg', '.png']:
        return url
    elif 'imgur' in url:
        return url + '.jpg'
    else:
        return False

u1 = 'http://i.imgur.com/jtboy'
url_is_image(u1)
print u1

Or you can replace return False with return None, depending on what happens next.

Improve this answer

Comments

-1

You must return what you have changed inside your function to take effect.

EDIT: following programming practices to not return different types, the previous code has been modified.

def url_is_image(url):
    exit_code = 1
    if url.endswith(('.jpg', '.png')):
        pass #exit_code still 1.
    elif 'imgur' in url:
        url += '.jpg' #exit_code still 1.
    else:
        exit_code = 0 #url does not match requirements
    return url, exit_code

u1 = 'http://i.imgur.com/jtboy'
u1, stat = url_is_image(u1)
if stat: #url has been modified or has required extension
    print (u1)
Improve this answer

7 Comments

I need the function to return True when the elif condition is satisfied. Is there a way to do that?
Do you mean only to return True in elif block or it can return something like a tuple (url, True) ?
I think returning a tuple is messy. There is no need to return two pieces of data. Instead of having a line somewhere that says if True, just say if not False or if url != None, which will just check to make sure the function did not reach the else
@jphollowed returning a tuple is no messier than returning a dict or any other object - and for this type of call it is perfectly valid
@gkusner I'm just saying there is no reason, when you can tell from the first piece of data whether or not it is an image or not. Returning a boolean is redudant and excessive. Though I suppose it is also more explicit.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.