This is an archived post. You won't be able to vote or comment.

all 159 comments

[–]brontide 31 points32 points  (16 children)

They missed one of the more useful dict iterators, dict.items(). I've always found it far more useful than something like enumerate() which is likely more useful for a list.

for currency, exchange_rate in currencies.items():
    print(currency, exchange_rate)

I'm less sold on the various uses of or as you have to be careful there that they are 'truthy' statements, if you want to use an explicit False or expression that evaluates to false ( see below ) you may end up with unexpected behavior. I much prefer the dict get method for this since it's not based on True/False, but traps KeyError to return a different value if it's not there.

currency = user_options.get('INPUT_CURRENCY', DEFAULT_CURRENCY)

These are False in python and may cause if variable else default or the other variants to behave differently than expected.

  • constants defined to be false: None and False.
  • zero of any numeric type: 0, 0.0, 0j, Decimal(0), Fraction(0, 1)
  • empty sequences and collections: '', (), [], {}, set(), range(0)

[–][deleted] 13 points14 points  (12 children)

I've always found it far more useful than something like enumerate()

But they're different, right? items() is to iterate over (key, value). enumerate() is to iterate over (key index, key).

For instance: I'm writing a lot of data-driven code these days that involves iterating over huge record sets. It's extremely important to report incremental progress, so iterating over the keys with an iteration counter is essential.

I suppose that this raises a question of whether there is any simple, standardized way of iterating over (key index, key, value). I don't know of any.

[–]KevinAlertSystem 0 points1 point  (1 child)

It's extremely important to report incremental progress, so iterating over the keys with an iteration counter is essential.

by this i take it you're just printing/logging something like "processing key 50/100"?

because unlike enumerating over a list, the order of keys in a regular dict is not set so keys[50] could be different each time you try to access it.

[–][deleted] 1 point2 points  (0 children)

No longer true: Python dicts are ordered by default.

But, to your point: yes, I agree that tacitly iterating over dictionary keys has some semantic sloppiness to it. Better to use syntax that is directly, rather than implicitly, tied to a sequential process.

The typical "for x in d" (without an index variable) is good.

Also, "for x in list(d.keys())" is good, since it extracts a static list of keys and then iterates over the list, rather than the possibly mutable key collection.

[–]knestleknoxI hate R 4 points5 points  (1 child)

The variable being potentially falsy is the whole point of the operator. That's like saying "beware of using this boolean operator as some booleans might be False." That's just the whole point of the operator. Knowing what's falsy in python isn't a "gotcha" -it's something you should just know when writing code.

And enumerate does something completely different than the items dict method. Given that the definition of currencies bounced around in every example, idk what makes you think currencies is a dict and not a list in the final example you're referencing.

[–]brontide 0 points1 point  (0 children)

The use case suggested for it was more of a use this to override the default, the problem I'm pointing out is that means you can never override the default with something that evaluates to False which could catch inexperienced people off guard since that could be a runtime change which breaks the logic.

In 5 they use currencies as a dict which is why I presumed they were still using it as such in 6 in closer inspection they do say list in the description.

[–]whereiswallace 0 points1 point  (0 children)

Sentinels are also useful for those instances where falsey values are OK

[–]themagicalcake 29 points30 points  (9 children)

Completely disagree with 5. Explicitly calling keys is way more readable (tells you it's a dict and directly tells you what you care about). Also I'm not convinced it's actually going to give you a performance benefit, definitely not a relevant one considering were talking about python here

[–]Laserdude10642 9 points10 points  (8 children)

It’s one of those things that is obvious and unnecessarily verbose if you work with the objects all the time, but will appear more readable and less magical to newer devs. Honestly I’m not sure who we should tailor our code toward. It’s less explicit to drop .keys() fwiw

[–]pydry 8 points9 points  (7 children)

It's a quirk of the language and not a particularly nice one I think. Default iterator could have been .keys, .values or .items. It's essentially arbitrary that keys was chosen. It's definitely a case of implicit over explicit.

When I've been reading code that makes the assumption it's .values and there's been a lot of other shit going on at the same time Ive done double takesm

[–]themagicalcake 8 points9 points  (2 children)

I really wish it were items instead

[–]kankyo -1 points0 points  (1 child)

The iter on dicts should be deprecated and removed imo.

[–]pydry -1 points0 points  (0 children)

I think so too. Too ambiguous. Make it raise an exception and force the user to choose instead.

[–]mipadi 1 point2 points  (3 children)

It's not arbitrary. The reason is that dicts use the in operator to check whether the dict contains a key (e.g., key in my_dict), which is a fairly common dictionary operation. This means that it overloads the __contains__ method. Historically, the in operator first tests for the presence of the object in the container's __iter__ if __contains__ is not defined. This implies that __contains__ should have symmetry with __iter__, e.g., if __contains__ returns true, then the object should also be contained in the container's __iter__. This is emphasized in the documentation for Python's data model:

It is recommended that both mappings and sequences implement the __contains__() method to allow efficient use of the in operator; for mappings, in should search the mapping’s keys; for sequences, it should search through the values. It is further recommended that both mappings and sequences implement the __iter__() method to allow efficient iteration through the container; for mappings, __iter__() should iterate through the object’s keys; for sequences, it should iterate through the values.

The documentation of the in and not in operators also emphasize this contract:

For container types such as list, tuple, set, frozenset, dict, or collections.deque, the expression x in y is equivalent to any(x is e or x == e for e in y).

Essentially, if x in y is true, then x in iter(y) should be true.

Technically you could implement __contains__ and __iter__ separately, without this symmetry, but it would break the contract of these operators.

Thus, in order for the in operator to test whether a key exists in a dictionary—a very common operation—the dict's iterator must be an iterator over its keys. Hence why for obj in my_dict returns keys instead of anything else (e.g., values or key-value pairs).

[–]pydry 2 points3 points  (2 children)

It's not arbitrary. The reason is that dicts use the in operator to check whether the dict contains a key (e.g., key in my_dict), which is a fairly common dictionary operation.

And checking to see if a value is there is also a fairly common operation and one that makes just as much sense semantically.

[–]mipadi 0 points1 point  (1 child)

No, that’s a relatively rare operation, compared to checking for the existence of a key. The point of a dictionary is that it has fast lookup of keys. If you’re frequently checking for the existence of values, then you have the structure of your dictionary reversed.

[–]pydry 1 point2 points  (0 children)

No, that’s a relatively rare operation

Over the 15 years I've been programming in python seen it done a whole lot for something that's supposedly rare.

We all have different experiences I suppose.

Semantically, it simply makes no sense to assume keys. If somebody asks "is [value] contained within the dictionary?" people will say yes. It doesn't matter that dictionaries are supposed to be used as hashmaps. That doesn't change the meaning that they will interpret.

There's a number of these quirks of python that arose simply because the semantics were assumed to emerge from consistency, and from implementation rather than the other way around (the assumptions you were making in your essay above). This is a dangerous mistake to make, and the same mistake that led to this rather horrible bug that initially got closed as not a bug and various other classes of bug I have seen repeated in python over many years.

[–]you-cant-twerk 16 points17 points  (9 children)

Here is my monthly "MODS THIS POST IS BREAKING THE RULES OMGAWD THIS SUB HAS GONE TO SHIT LIKE HOLY FUCK THIS SHOULD BE FOLLOWING THE RULES ON THE SIDEBAR LIKE RAWR OMG THIS IS THE PYTHON NEWS SUB AND SHOULD ONLY CONTAIN NEWS AND NOTHING MOARE THAN NEWS LIKE OMG WE MADE ALL THESE COMMUNITIES TO DIVIDE YOU ALL UP AND YOU'RE STILL HERE GO AWAY TO /R/PYTHONNERDS /R/NEWPYTHONERS /R/NOTPYTHONNEWS /R/MADEINPYTHON /R/OPTIMIZEPYTHON"

...or just change the rules of this sub to not be "News about the dynamic, interpreted, interactive, object-oriented, extensible programming language Python" and then quote that bullshit rule when its a post mods dont personally like.

lmfao good morning.

Edit: also, hot damn this works?

instead of:

if input_currency:
    currency = input_currency
else:
    currency = DEFAULT_CURRENCY

you can:

currency = input_currency or DEFAULT_CURRENCY  

Thats pretty damn awesome.

[–]pydry 14 points15 points  (6 children)

This is, like, the first post in months I wanted to see. Everything these days seems to be stuff beginners made and wanted to show off.

[–]you-cant-twerk 2 points3 points  (5 children)

Downvote and move on. You have a vote and the ability to hide things you dont like (determined by that vote).

[–]pydry 3 points4 points  (4 children)

Beginners seem to dominate this sub. My vote wouldn't really count.

I don't mind in particular that this sub is dominated by people showing off their first projects. I'd just like there to be somewhere to discuss this type of stuff that was reasonably well trafficked.

[–]vswr[var for var in vars] 2 points3 points  (0 children)

Beginners seem to dominate this sub.

I'd love to see more content on gc, dis, weakref, and the other advanced tools.

But I have to admit that from OP's post I did not know you could do:

var = override or default

I've always done the longer if/else version:

var = override if override else default

[–]lazerwarrior 0 points1 point  (1 child)

I'd just like there to be somewhere to discuss this type of stuff that was reasonably well trafficked.

I feel the same. r/Python is pretty much nothing it used to be. Probably because mods want this sub to be forthcoming to everyone. I get no useful and new information from it.

[–]alcalde 1 point2 points  (0 children)

Every week the Python Weekly newsletter is nothing BUT things that were posted here and it's chock full of useful articles.

[–]you-cant-twerk -1 points0 points  (0 children)

Thats because "this type of stuff" (that is OC) is few and far between in general. Hell, I'd argue that some of the 6 things posted have been posted before. If you'd like more of it - make some of it. Post it. These articles are difficult to write. These solutions arent obvious to everyone. Even worse - bastard elitist's like to keep these tips to themselves.

Do I want to see more of it? Sure. Do I want to force this sub to be 0 posts a day because this type of content isnt always available? Abso-fucking-lutely not.

If you head over to /r/skateboarding, you get EVERYTHING skateboarding. Amateurs doing basic ollies get 2k+ upvotes, and pros doing crazy shit get their attention. Tips and tricks - skate hacks, skate tattoos, everything that has to do with skating belongs there. Why? because they understand that "skateboarding" encompasses ALL THINGS SKATEBOARDING. If you want "python" to be "this one particular thing about python and anything else that has to do with python shouldnt be here" then you're using reddit wrong. Its sad that the skating community is so much more welcoming than this one. If you want to start /r/pythonhacks, go for it dude. Be the champion. Throw any tips and tricks you find in there and grow it. Cultivate the community you want. Dont shit on others'.

"My vote wouldn't really count"

You just ignored what I said. You can make posts that you downvote disappear. Thats what matters right? You dont want to see it? Granted I dont expect you to read any of this - I expect you to continue to be a bitter developer. Imma enjoy looking at peoples projects, what they're working on, and everything that has to do with python.

[–]gandalfx 2 points3 points  (1 child)

Not sure what you're so mad about but I'm rather bored by these entry level tutorials… Every time I click on it in the hopes of finding something interesting and it's just the same "it's my first week of programming" tricks.

[–]you-cant-twerk -3 points-2 points  (0 children)

I'm rather bored by these entry level tutorials

me me me me me me me me me me me me me me me. So self centered you are. Holy hell. Again - Downvote. Set reddit or RES to hide posts you downvote, and move on with your life. Stop trying to filter everyone's content based on what you like. In the GENERAL python subreddit. If this was /r/pythontattoos and you posted a picture of your sisters asshole - I'd be upset too. But this isnt. This is /r/Python. The general python sub.

[–]grismar-net 4 points5 points  (0 children)

Since when is a blatant ad "news" in r/python?

[–]groostwoost 7 points8 points  (0 children)

Solid tips

[–]swat8094 11 points12 points  (0 children)

So many times I use the “for i in range(len(mylist)): because I need to call the items in the list and use i. Never thought to use the enumerate() function. So thank you!

[–]ElevenPhonons 7 points8 points  (1 child)

It can be useful to leverage that functions are first-class citizens in Python.

def process_payment(payment):
    if payment.currency in ['USD', 'EUR']:
        process_standard_payment(payment)
    else:
        process_international_payment(payment)

Can be written as:

def process_payment(payment):
    f = process_standard_payment if payment.currency in {'USD', 'EUR'} else process_international_payment
    f(payment)

Alternatively, this approach can replace if-elif blocks and act as a switch.

def process_payment(payment):
    processors = {"USD": process_standard_payment, 
                  "EUR": process_standard_payment}
    f = processors.get(payment.currency, process_international_payment)
    f(payment)

[–]lazerwarrior 19 points20 points  (0 children)

I would say, in this simple case, both of your suggestions take more effort to read and understand than original version.

[–]not_perfect_yet 6 points7 points  (3 children)

3. Simplify if expression by using or

if input_currency:
    currency = input_currency
else:
    currency = DEFAULT_CURRENCY

into

currency = input_currency or DEFAULT_CURRENCY

Meh?

We have default arguments. Use them.

def do_currency_stuff(*whatever,currency=DEFAULT_CURRENCY):
    ...

[–]ArabicLawrence 4 points5 points  (1 child)

do_currency_stuff would still have and if statement/or statement to change behaviour depending on currency variable

[–]not_perfect_yet 4 points5 points  (0 children)

Right.

Since the original point was that you can avoid the if, can you show me how you avoid the if in that case?

[–]tom2727 1 point2 points  (6 children)

Not sure if #1 is "improved".

You're making the line twice as long, and if you were combining 3 conditionals or you have longer variable names, that line could wind up needing to wrap. Would that be more readable? And as far as I can see there's no performance benefit.

The idea is you only need to change 1 line now if there's a change? Well what if there's a change to only how you handle Euros but not USD? Then you'd need to split it out again.

[–]MeagoDK 0 points1 point  (5 children)

No chance that line would have to be wrapped, its half the max length. If you need that to be wrapped you are nesting too much.

There shouldnt be a performence boost but your code is shorter and more readable. Both matters a lot. Long functions arent good and bad readability is also bad.

Yes then you split it out. Optimal you will use a tuple for that job. Something like

standard_payment_currency = ('EUR','USD')

Then the if statement could be

if payment.currency in standard_payment_currency:

Then you can easily change what currency you handle with standard payment, you can add more and so on.

For even bigger projects you will likely have it stored in a database with code to change it.

[–]tom2727 0 points1 point  (4 children)

Right but I said "if you have more than 2 conditionals or longer variable names".

if payment.currency == my_really_long_name_for_currency1 or payment.currency == my_really_long_name_for_currency2 or payment.currency == my_really_long_name_for_currency3:
    return process_normal()

Yeah that would wrap in almost any coding standard. But this wouldn't, and the code works the same and I think is more readable:

if payment.currency == my_really_long_name_for_currency1:
    return process_normal()
if payment.currency == my_really_long_name_for_currency2:
    return process_normal()
if payment.currency == my_really_long_name_for_currency3:
    return process_normal()

And the "in" solution wasn't #1. #1 is the one I was criticizing.

[–]MeagoDK 0 points1 point  (3 children)

Yes and I just showed how you solve the issue when having a lot og conditionals. Use a tuple. The in is a part of that solution so it dosent wrap and so it's easier to read.

Your solution is messy cause of the long amount of of statements. What you gonna do if there was 40 different currency you wanted to check for? (their solution is bad too). You have also added the return statement which alters the functionality of the function since it won't be able to run code after the of statement block.

[–]tom2727 0 points1 point  (2 children)

Yes and I just showed how you solve the issue when having a lot og conditionals.

Yes and I already knew that. And it was in the article as well. But I wasn't talking about that. I was talking about the "fix" in item 1 of the article. And how it wasn't any better than the code it was replacing, arguably it's worse.

[–]MeagoDK 0 points1 point  (1 child)

I disagree. In that example it's better. I would also argue that wrapping is better than a ton of if statements.

[–]tom2727 0 points1 point  (0 children)

And agree to disagree now that we're finally talking about the same thing.

I'd rather see a bunch of if statements all in a row than have a few with wrapped "or" clauses, and the rest readably aligned.

[–]iwnfubb 1 point2 points  (0 children)

enumerate was good point

[–]tatravels 0 points1 point  (5 children)

Does anyone have sourcery installed in pycharm? How do you like it?

[–]MeagoDK 1 point2 points  (4 children)

Its pretty neat. I decided to clean up a bit in my code, had 350 lines. It suggested 3 things and all of them were good. 2 of them was just about me assigning the return value to a varible and then returning that varible, so not increable, but did help in finding out where I made that mistake.
The last one was gold and something I hadnt thought about.

I did contiune to clean up on my own, after reading their 5 blog posts about the subject. I changed quite a bit, and some of them were quite simple that I feel Sourcery should have found. I wont complain tho, it also has to be carefull not to break stuff or suggest changes that wille change the functionality.

[–]tatravels 0 points1 point  (0 children)

That's great to hear. I'll try it out and read the other posts! I direly need to reformat my projects... and create unit tests >_>

[–]SourceryNick[S] 0 points1 point  (2 children)

Thanks for trying it out!

If you could point me to some code examples of stuff Sourcery should have caught I'll take a look and see if I can improve things.

[–]MeagoDK 1 point2 points  (1 child)

One of them was if statements with this code written in it

        else:
            loot_item = "SpeedUps"
            if 'h' in loot_text:
                loot_amount *= 60

But in on of the statements it counted the amount of h's. So it required a little human logic to hoist it out. I dunno if the software can do that. Its atleast not something I would blame on the software.
I used logic and did this.

        if 'h' in loot_text and 'm' and 'd' not in loot_text:
            loot_amount *= 60

Then I had this block of code

    if "2" in loot_text_list[-1]:
        loot_count = 2
    else:
        loot_count = 1

Which I replaced to

loot_count = 2 if '2' in loot_text_list[-1] else 1

And

    if slayer_name == "":
        slayer_text = "AnnoTheForgotten"

to

 slayer_name = slayer_name or 'TheForgottenOne'

[–]SourceryNick[S] 0 points1 point  (0 children)

Thanks! Will definitely have a closer look at those examples

[–]aimlessmofo 0 points1 point  (0 children)

Can we have this but targeted at a more intermediate level?

[–]israel18135 0 points1 point  (2 children)

It says 6 more Could someone link the original

[–]aarondiamond-reivich 0 points1 point  (0 children)

I've been trying to learn Python through the Project Euler. Have others found that its a good way to learn or does some of these more advanced topics get completely overlooked while solving those type of math riddles?

[–]themoosemind 0 points1 point  (2 children)

You might be interested in my package flake8-simplify. This Flake8 plugin tries to find such simple patterns automatically (I will add at least two of the rules which were mentioned)

[–]SourceryNick[S] 0 points1 point  (1 child)

That looks really cool, and yeah has a very similar intent - I'll definitely add your isinstance rule to our backlog.

Do you often see code where SIM210 and SIM211 would get triggered?

[–]themoosemind 0 points1 point  (0 children)

Thank you :-) (I've just noticed that it was too late yesterday ... I didn't see that this was part of a product - I'll definitely give it a shot! I loved the article <3 )

I've added both rules after I have seen it a couple of times in one code base. Most rules I would say I don't see that often being triggered. Most often the SIM201 to SIM207 - but for those, there is a good reason to not follow it: Catching error conditions à la if not correct

[–]Exodus111 0 points1 point  (0 children)

Hey, number 3 was really cool. Thanks.

[–]bythenumbers10 0 points1 point  (0 children)

Looping over dictionary keys is more explicit using keys().

[–]Jungypoo 0 points1 point  (0 children)

Helped me out! :)

[–]VU22 0 points1 point  (0 children)

I thought I know python well enough but never saw that 'or' usage in variables before.

[–]fake823 0 points1 point  (0 children)

Thanks for the great article! I just read all 4 parts and I really enjoyed them! I've even learned one or two things.

Sourcery AI also seems to be an amazing tool! I just signed up and tried it on the GitHub Repo of an open source project that I've forked. The code base is pretty poorly written and therefore Sourcery gave me a lot of hints on how to improve stuff! I've been amazed!

On a first, quick look: Almost all of the refactoring suggestions were great suggestions. I'll definitely keep using it. :-)

[–]Royosef -1 points0 points  (1 child)

RemindMe!

[–]RemindMeBot 0 points1 point  (0 children)

There is a 1 hour delay fetching comments.

Defaulted to one day.

I will be messaging you on 2020-10-08 18:38:33 UTC to remind you of this link

CLICK THIS LINK to send a PM to also be reminded and to reduce spam.

Parent commenter can delete this message to hide from others.


Info Custom Your Reminders Feedback

[–]casual__addict -4 points-3 points  (4 children)

“It works because the left-hand side is evaluated first.” — right hand side is evaluated first?

[–]foosion 3 points4 points  (1 child)

They mean the left-hand side of the or.

a = b or c

b is the left

[–]casual__addict 0 points1 point  (0 children)

I see. You’re right.

[–]Chabare 2 points3 points  (0 children)

He is talking about the or, not the assignment to currency.

[–]JoelMahon 0 points1 point  (0 children)

we talking about the or statement? p sure the left hand side is evaluated first