Google Python Style Guide
google.github.io
google.github.io
Defaults are useful when you are providing a library function for other teams to use. If you’re inside a more private code base and doing work on the implementation of your team’s service then it is wise to avoid default arguments.
The problem is they provide a point after which it seems acceptable to add a flood of more default arguments. This is particularly the case for junior developers who lack confidence to refactor instead of patch. Default arguments go hand in hand with conditional logic and cause functions to bloat into do-everything multi-page monsters without any focus and no tractable flow of logic.
Forgive the contrived example, but what was once this:
def greet(name):
print(f”Hello {name}”)
ends up becoming this, all because no one would bite the bullet and pick this apart into individual functions: def greet(
name,
language=None,
io=None,
is_ci=False,
and_return=False,
):
greeting = “Hello”
if language:
greeting = translate(greeting)
message = f”{greeting} {name}”
fn = print
flush = False
if is_ci:
fn = log
flush = True
fn(
greeting,
flush=flush,
io=io if io else stdout,
)
if and_return:
return greeting
The slow rot of more and more defaults makes the function longer and longer. Moreover, each time someone adds a new option it gets harder to justify why they shouldn’t do it when the previous person was allowed.To me it doesn't illustrate the problem with default parameters specifically.
For example it shows that the programmer doesn't know dependency injection and first-class functions. Printing could be passed in as a function param, but then it might actually be sensible to provide a default callable (eg, print), depending on how the greet function is going to be called.
Language seems to be a perfectly sensible thing to have a default on.
Then, and_return... That is, like, super contrived, man... I mean if a programmer doesn't know that a function caller can simply call for side effects and ignore the return value, they likely have much bigger problems than their judgement to use defaults or not.
I empathize with your plight though - you're probably a great programmer, and I think it's very difficult for someone who is good at their craft to come up with genuinely but subtly shitty examples.
def greet(…, bow=False):
…
if bow:
take_a_bow()
Except imagine take_a_bow() instead as 10 lines of code to perform the post-greeting bow-ceremony. That code takes additional arguments regarding what kind of hand flourish to perform while bowing. The flourish_type has to be an optional argument to greet (because bow is) but inside the bow you have to assert flourish_type is not None because you can’t bow without knowing what flourish to give.I’ve seen some dark stuff over the years.
Also: Welcome to HN!
def greet(bow=None):
...
if bow:
bow()
def elsewhere(gesture):
def bow():
print(gesture)
greet(bow)
Wrap that.Also stop being so religious and defensive when somebody mentions things that are not standard in your language of choice as nobody forces you to use this.
I was sucessfully building very testable and maintanable codebases (~50 kloc) in Python while also using very small in-house built DI framework and it was subjectively (by me and my collegues) much better than what we had before while we followed standard Python patterns and ways Python frameworks teach you to follow.
Can I guess you were all Java developers?
I still love using Python for REPL, small scripts or prototyping, and I think having things like mypy is great as it takes away much of the burden without being a huge obstacle in some situations where you really need to use duck typing. Also I'm thankful that it teached me early that the debugger is one of the developer's best friends and the best documentation is just reading the code.
When that kind of “handy defaults for ya!” programming happens to a function inside a package… that has four call sites… all of which were added by the same team of three people… just refactor your stuff, get functional, and say explicitly what you actually need.
But at the same time I wonder how it would look refacotred. How many read from csv functions would we be left with?
It probably couldn't be that, because many build on one another. Some are deprecated and others are clearly incompatible, but out of 50 parameters you likely could imagine calling this with 20 parameters if the environment and the CSV you're ingesting are wonky enough.
I think feasible refactorings would be:
- rationalise currently separate parameters into meatier objects e.g. there's at least half a dozen parameters which deal with dates parsing, a dozen which configure the low-level CSV parsing, etc... that could probably be coalesced into configuration objects
- a builder-type API, but you'd end up at the same result using intermediate steps instead of a function, not really useful unless you leverage (1) and each builder step configures a non-trivial amount of the system, so rather than 50 parameters you'd have maybe 10 builder, each with 0~10 knobs
- or you'd build the thing as a bunch of composable transformers on top of a base parser
Of note: the latter at least might be undesirable from the Pandas POV, as it would imply layers of recursive Python calls, which might be much slower than whatever Pandas currently does (I've no idea).
``` pyarrow.csv.read_csv(input_file, read_options=None, parse_options=None, convert_options=None, MemoryPool memory_pool=None) ```
You can then pass a `ReadOptions`[1] object if needed.
For example:
``` read_options = csv.ReadOptions( column_names=["animals", "n_legs", "entry"], skip_rows=1) csv.read_csv(io.BytesIO(s.encode()), read_options=read_options) ```
You can see how ReadOptions is written on this link [2]. It's interesting they use a `cdef class` from `Cython` for this.
This doesn't solve all issues (the ReadOptions object and the others will inevitably have a bunch of default arguments) but I do think it's safer and it's easier to have a mental map of the things you need to decide and what's decided for you.
[0] https://arrow.apache.org/docs/python/generated/pyarrow.csv.r... [1] https://arrow.apache.org/docs/python/generated/pyarrow.csv.R... [2] https://github.com/apache/arrow/blob/master/python/pyarrow/_...
Adding default parameters works well with existing code. It is not bad and lazy because it is easy.
If greet() gave up responsibility for outputting the message and instead just constructed it, then your code would look like this:
def site1():
print(greet(“x”))
def site2():
log(greet(“y”))
And greet would be half as long.def site(message, port=print): port(greet(message))
if and_return:
return greeting
Is a nice touch. I've definitely seen that pattern in the wild.Then it’s even worse when there’s also an “allow_raise” param, where the return type will not be None if allow_raise=True. Now you need to write 4 overloaded signatures to account for the 2 polymorphic params
I work on a very large codebase like this and most functions return lists of strings/tuples and most DTOs will be dictionaries with string keys. Instead of classes which have methods to retrieve information in different ways. Therefore parameters have been added to return information in more and more ways.
I’m not sure languages should be limited in order to avoid problems with the lack of project leadership.
In my opinion, function should list its dependencies and allow changing them. Having said that I dont believe the `is_ci` decision should happen in the function. The decision should happen at the entrypoint and it should drive which implementations the code will use for the dependencies.
I would look for the reason of rot in making the function become the merge point of multiple context, not the default values per-se. Whether default arguments make merging multiple contexts in a single function easier - code reviews might help here.
In any case, very good example
Others also mention pandas
Data science workflows in general love to do this
https://github.com/charliermarsh/ruff
This article also highlights some pain points I have with python. They say avoid big list comprehensions, but there is no nice way of piping data in Python one can switch to instead. Especially with lambdas being so underpowered (so they also say to avoid).
They say avoid conditional expressions for all but simple cases, and I agree. Which is what makes me wish everything (like ifs, switches/when etc) in Python was an expression (ala elm, kotlin etc). Because right now it's hard to assign a variable conditionally without having to re-assign / mutate it, which feels unpure.
Default arguments being reused between calls is just weird. So I understand the rationale of Google's style guide, but it's a big flaw in the language imo, lots of weird bugs from novice programmers from that.
I disagree on allowing the @property decorator. You think you're doing a lookup, but it's suddenly a function call doing a db lookup or so. Huge foot gun.
I feel the 80 character rule is too low, and often hard to avoid with keyword arguments, to the django orm etc. End up having to litter comments to disable it all over the place, and waste a lot of time when CI breaks.
As for formatting, whitespace etc. I'm over caring about that for languages. I just have some autoformatter set up to fix everything on save, and someone else can argue about the rules.
Write generator functions, i.e. functions that yield their results. They are surprisingly powerful. I usually find I need only one or two.
I agree that ruff seems to be the way forward, it's (almost) at feature-parity, it is extremely fast, but I think it needs polishing
So I've actually seen little use of match cases so far.
E.g.,
def bla(self, a, b, c=2, d=True):
self.bla("x", "x", 3)
Ain't no way in hell I'm wanting those arguments to be used positionally by callers. They're getting THE KWARG IS MANDATORY STAR. def bla(self, *, a, b, c=2, d=True):
self.bla(a="x", b="x", c=3)
It adds more vertical to your code when you have meaningful argument names, but it makes it a lot clearer what is what, and the language enforces what just used to be a good style.It's especially important when Python's mocking comes into play.
In languages like Java, you can determine what is what based on types... (with static imports for both)
X x = new X(mock(Y.class));
Makes it far clearer what X is working with than x: X = X(MagicMock())Also, Java’s a pretty bad example, as it lacks default arguments and named arguments, leading to the ugly and verbose builder pattern. (Side note, why didn’t Java add them yet, after so many years of their usefulness being seen in Scala, Kotlin, C# to name a few related languages?)
x: X = X(MagicMock(Y))
x: X = X(MagicMock(Y(...)))
x: X = X(MagicMock(spec=Y))
which all make it fairly clear that the Mock is of Y. (or better yet, use fakes instead of mocks)Sure you can objects bits it not the same.
But one nice thing is that consumption is less verbose than Python. In Python you write “greet(name=name)” but in TS you write “greet({name})”
2.14 True/False Evaluations
Use the “implicit” false if at all possible.
This one is my personal bug-bear. I find this: if not users:
...
significantly worse than: if users == []:
...
The second is totally explicit, reminds the reader that users is (expected to be) a list and makes it totally clear that we can only enter the conditional block if users is an empty list.The first option:
a) obfuscates the type of users on first reading
b) evaluates to True if users is None (or LOADS of other things?!) which can lead to hard-to-find bugs.
Granted, type-checking can help here but purely from a readability perspective the second option seems way more friendly and for almost no downside. The same holds true for all of the "False-y" objects:
if users == {}:
if users == 0:
if users is None:
if users == ():
if users is False:
Why is the implicit: if not users:
an improvement in any of these cases? If you need to distinguish False from None then chain the expressions, such as if not x and x is not None:.
!!!Why not just:
if x is False:
?This goes for if you make a library as well, your library will be easier to use if you are less strict about the inputs you take, since that allows your user to work in a more naturally dynamic way. I love static types, but I have worked on making python libraries and there accepting a wide range of inputs is an important part of usability.
Each to their own liking, I prefer knowing what argument types a function accepts so I don't need to think about it, and focus on writing business logic. If the function could accept more types, Id just improve it.
That is for library code, maybe it would be too cumbersome to try to do that for code with less reuse. I have never worked on a large python codebase that wasn't a library so I'm not sure what is best there.
For beginners, and in toy examples, it's kind of neat when code bends over backwards to work. Take this example:
>>> def all_uppercase(lst):
... return [s.upper() for s in lst]
...
>>> all_uppercase('hi')
['H', 'I']
>>>
Kind of neat, right? It's almost like a joke in code. Ha ha, iterating over a string gives you strings! But the charm of finding cute things to do with unexpected inputs doesn't scale. What is a helpful attitude at a small scale translates to "errors should manifest as far away as possible from the programming mistake that caused them" at a large scale. 99 times out of 100, if your code expects a list and somebody passes a tuple, they want a stack trace, not a return value.> I have worked on making python libraries and there accepting a wide range of inputs is an important part of usability
I work with a large Python codebase at work, and this is a frequent source of frustration. I frequently track down bugs and find that on some untested code path our code passes nonsensical values of the wrong type to a third-party library, and the library just... finds some way of interpreting it.
Even if all the code paths get tested, they can't be tested with every possible input. Property-based testing seems like overkill for our application, and our tests are already almost slow enough to be annoying. And what if the third-party library is side-effecting in a way that's hard to test? It gets mocked out. And I find that the mocks are configured to expect the nonsensical values, because the original programmer found that they "work."
All because libraries don't want to make an unfriendly impression by throwing a stack trace.
> if not users:
> an improvement in any of these cases?
Function iterates over the input. User provides a list,
if users == ():
test fucks up because lists and tuples are never equal.Literally no gain, only pain.
Wouldn't this just be the developer using the wrong comparison for the types the function is expecting(hence more reason to be explicit instead of using the implicit false)?
Because for most functions that's not a relevant or useful distinction, in Python a tuple is an immutable list, both are sequences.
By mis-handling empty tuples you're just unnecessarily constraining the caller. Not only that, but you might also create an inconsistency which is hard for the caller to notice if your function only fucks up on empty collections.
The generalization of this is to code against as generic an api as possible, you wouldn't do `list.__eq___(x, y)` in your code, but you're suggesting almost exactly that.
(granted you can still run into this kind of issue if foo is a generator, but that's a less common way to explode).
The style guide does tell you to use explicit `x is None`, instead of implicit bool when checking noneness, specifically to disambiguate between binary and ternary values, but usually that's not what you want.
If we use Python as a strongly typed language, it makes no difference which one you use.
If we don't (i.e. use Python as it is: a dynamically-typed language), then this is just a preference.
Using (or exploiting, depends on how you think) Truthiness this way is actually an intentional choice in lots of case, especially if you have "else" condition.
Think it this way: you're going to split the conditions into two: `users` is non-empty, which is the "good" condition; and `users` is empty, which is the "bad" condition.
Then you have unexpected condition that "users" is something that shouldn't be, most commonly being None. In most of cases, this is a "bad" condition. So it makes sense it's grouped together with `users == []`.
If `users` is "True" or "False" as you said (which you should ensure to not happen in other ways anyway), then indeed it will not be captured by `users == []`, but it would still be broken/unmanaged in "else" side.
Nitpick: Python is strongly typed, it's also dynamic. The strongly-weakly typed axis is different from the static-dynamic axis.
In a lot of cases though, an empty collection isn't a "bad" condition at all, e.g. it's a valid collection to apply filters/maps to.
Similarly when people get used to doing "if not i" for ints, but then forget about the times that zero is a valid value.
It's true that dynamic coercion is a feature of the language, but coding conventions generally are often about enforcing "least surprise" to remove a burden from the person reading the code.
Then you don't need to check if it's empty to begin with.
An empty list could be valid(e.x. a search of users providing no results) so you still need to differentiate between "special cases that need special logic" and "bad input".
Lumping the two together in one `if` block makes any code less readable imo, because they're not the same thing.
Is an empty list being routed to the else branch because it is an error in this instance or because it's an error in 90% of the codebase so the author forgot to handle it explicitly here?
Or is the author always expecting users will be a full or empty list and that other falsy values will never occur?
If there is a bug and somebody passes invalid type to my function, I would just fix it and move on.
Very often both None and empty list are not an interesting case and I return early. Thus `not users` makes sense.
There are cases, though, when None means a sane default should be used instead and you can't use default argument values due to mutability. In these cases `users is None` makes perfect sense.
There are also cases I explicitly check for True and False - tests. In such cases I wouldn't rely on truthy/falsy values and assert True and False values by reference.
Having said that, its all subject ive. You like this style, some one else likes other style. What ultimately matters are two things: automatic formatters and consistency.
Others have mentioned that the comparison to say a tuple will also fail. If the intent is to ensure a list instance, use isinstance, instead.
if users == []:
Nor is this: if (users == []) == True:
Nor is this: if ((users == []) == True) == True:
.../s
Probably the worst thing about this one is the pydoc. An not just pydoc. Pydoc, javadoc, doxygen. It is all useless garbage that litters the code with useless comments explaining that the get_height method "gets the height" while at the same time nobody is actually explaining anything remotely useful in comments.
Sure, we can discuss whether all poiktsw make sense but it seems excessive to dismiss them entirely just because you dislike mandatory method docstrings.
So besides the useless comments I also have that experience described in the previous paragraph.
And to top it all off, another 'nice' experience with style guides was being forced to used the long discredited 'hungarian notation' which at the time was already long past its expiry date with not really anyone competent believing it to be a good thing.
Maybe my bad experiences are not typical but I have had so many bad experiences with style guides that I am now at the stage where I consider anyone enforcing a style guide to be my personal enemy and most likely a despicable person.
There probably are ways to encourage good comments but enforcing somebody write A comment, ANY comment in specific places at the point when theyre eager to merge is pretty much a recipe for shitty comments.
Also, the requirement to have documentation comments is just one rule out of hundreds, so why focus on that one?
Just as with comments I think you should assume the reader is a somewhat competent human beeing. Explain what needs to be explained, and make a *conscious* decision about it. For most public functions I agree that this usually involves a docstring, but not always.
Why focus on that one? Because it turns out that rule will force me to have 50% of the text that I see in a file be these useless comments.
def get_height(): # noqa
…
Is just as annoying as useless documentation.># BAD COMMENT: Now go through the b array and make sure whenever i occurs
># the next element is i+1
In practice style guides will usually advise against the kind of comment you're blaming them for.
Does get_height() include the optional border width or not? What does it return while the item is hidden? Does it return an int or float? Can it return a string value like "auto" or a null value? Under what circumstances should its value not be relied upon, e.g. while an animation is active? Why was it written as a function rather than a property? And so forth.
Requiring a comment is helpful in reminding the author to explain why this code was even separated out as a separate function in the first place, and all of the potential gotchas around it.
(also use numericalunits if you have so many dimensions that they're easily mixed up)
class Rectangle: ...
def get_height() -> Micrometer:
....
Now we clearly need to sufficiently document this method by saying "gets the height of the Rectangle in Micrometers".And yes, this is the garbage kind of answer that one gets whenever one brings up the point that I brought up. Honestly, I am already fondly hoping you will never be a colleague of mine.
This kind of 'documentation' has to be the worst and most disgusting kind of cargo culting ever invented in programming.
Personally, I’d agree with your example. But does this apply to all other situations? Often documentation comments can be very useful
Even for e.g. get_height, does it make a DB call? Is it expensive and shouldn't be called in a tight loop? Can it ever return 0?
if get_height returns a simple value, then it "smells" like it should be a simple getter.
So regardless of your comment, people will use it exactly like that and get burnt. Your comment only helps once they're already suffering enough to go digging for why things are so slow.
If the operation needs a DB call it should be exposing a way to handle the infinitely higher likelihood of failure with something like a result type, and it probably shouldn't be a simple blocking call
-
No one is saying you should never have comments by the way, but pydoc-style guidelines where you're encouraged to fill out a template end up with terrible signal to noise.
Comments should be seen as an absolute last resort and a liability.
The only parts of comments that can be automatically refactored are the absolute least useful ones. Things like intent aren't magically be pulled out and updated automatically (yet), so you're now putting the onus on every single person who ever touches that code again to keep your comment up to date, otherwise it's can end up worse than nothing.
I worked on a team like this who regularly wrote function names well into the 160-200 character range. It was useless.
There are some refactorings that Sourcery suggest that I don't agree with myself, namely the usage of 'contextlib.suppress'[4] as I don't like to introduce an additional 'import' statement just to do something so trivial. I wish Sourcery would add the relevance of having possibly too many 'import' statements as a heuristic.
---
[1]: https://sourcery.ai/
[2]: https://docs.sourcery.ai/Reference/Default-Rules/ (expand the sub-pages)
[3]: https://docs.sourcery.ai/Reference/Optional-Rules/gpsg/
[4]: https://docs.sourcery.ai/Reference/Default-Rules/refactoring...
Python Style Guide from Google - https://news.ycombinator.com/item?id=11839332 - June 2016 (27 comments)
Google's Python style guide - https://news.ycombinator.com/item?id=3861617 - April 2012 (86 comments)
Google Python Style Guide - https://news.ycombinator.com/item?id=1311126 - May 2010 (23 comments)
[1]: https://google.github.io/styleguide/pyguide.html#217-functio...
Class methods are a weird thing that can be used to affect all objects of a class, so are kinda global in a sense, which is why they should be avoided.
If you have a class like this with a separate constructor in the same scope as the class…
class Cow:
def __init__(self, name)
…
def random_cow() -> Cow:
return Cow(uuid())
…you are more likely to roll this all up into farm.cow than you are to lump all the animals together in a single farm module.Modularity is nice of course because it helps you step away from implementation detail (close the file, forget about how it works, and just use it) and your code gets split up into little pieces that helps your e.g. bazel monorepo build/test work efficiently.
In fact, a better article would have been to try and predict the eye powers of a programmer based on their formatting choices.
Edit: Did someone seriously downvote me for how many spaces I indent with? Funniest downvote I've gotten so far.
I think all these will be covered by the "be consistent" clause, and whoever made the first commit decides the style.
* no goofy animations (no slide down when the table of contents expands, just BOOM open)
* no distracting images / icons
* no dark mode toggle (blasphemous nowadays, I know...)
* black and white
* logical font sizing for section / sub section numbers
Just clean and simple. My compliments to the designer.
Why the hate ? I love an option for dark mode or else I go blinded by the lights on my monitor.
https://github.com/google/yapf/blob/v0.32.0/yapf/yapflib/sty...
The exception to this rule could be functions that have only one argument.
The guide states this is unclear:
import jodie
I agree, but why not using: import .jodieBeing a mostly Lisp developer, I like to nest local functions in order to close over locally defined variables. Glad that is considered OK in Python.
In reviews I always ask that there must be a separate formatting commit, at the end.
Also, because our builds fail if the code is not formatted, that means constant reformatting and moving around of commits.
In the end the time wasted to start the container to run black (if you use the distribution one, every version formats differently), to run black (which is terribly slow), and juggle the commits around is hardly worth it.
However I believe from a management perspective it gets rid of discussions about style in the reviews, so it looks like time is being saved because now the developers waste it each on their own in silence, without communicating.
We had an internal debate about how to gauge code quality. One camp only allowed the combination of black format plus coverage. To play devils advocate, I said that the number of asserts removed or added per merge request.
And this defeats trying to reuse variable names from publication, so that a more-international audience can follow ‘ss’ rather than sum_of_squared_residuals_dude.
Julia promotes this as a war cry over Python.
That's nice but not everybody thinks that way, and when the project gets big enough inconsistencies get introduced everywhere so there is no way to be "consistent with the codebase" unless you impose consistency via a style guide (and ideally a linter, so reviewers don't have to be the one performing style checking)
I understand that it make the code less verbose, but I don't agree that is "less error prone" as stated.
> May look strange to C/C++ developers.
Yes..
“2.5 Mutable Global State
Avoid mutable global state.
[…]
2.5.2 Pros
Occasionally useful.”
So, are these pros of mutable global state or of avoiding it? Elsewhere pros and cons appear to refer to the description of the guideline, not the title (see 2.2 Imports).
2.5.4 Decision
Avoid mutable global state."
This style guide touches both pros and cons then states the decision.
Honestly even with a version manager it can become a nightmare and it’s the primary reason I’ve stayed away from it. Also because I’m really a mathematician or something who needs to use any of the extensive python libraries to do some cool AI stuff.
Here’s to hoping I never have to deal with actually maintaining or working on a python codebase. Cheers!
You should file a bug report. Sounds like an über P1 because of the physical workstation damage.
But I meant more that even tho I was using pyenv somehow broke my gcloud cli and took me a few minutes of frustration to get things working again because I had to install some other dependencies I didn’t have. Eventually I just ended up using it in a vm. :/
Not exactly the most pleasant dx compared to the other programming languages I work with on the daily.
2. activate virtualenv: . bla/bin/activate
3. install stuff: pip install blablablabla
4. do things
5. remove bla and repeat if you want to start clean
> Here’s to hoping I never have to deal with actually maintaining or working on a python codebase. Cheers!
In this case, just use distribution packages.
I think it’s actually great that in Python you can still call these functions in a pinch.
Sometimes you've got internal code. Sometimes people use it. Communicating which code is internal helps people avoid depending on it. Allowing people to access internals allows them to decide if it's worth the risk, and sometimes it is worth the risk. Tooling support makes it easier to work with this model.
Prefixing module variables with an underscore is a bit strange.
Type hinting could do it today but you’d need a PEP to add it.
It's a pretty standard setting in every editor I've used, including vim.
Do you use notepad.exe or something more ancient like edline?
https://docs.godotengine.org/en/stable/tutorials/scripting/g...
I have come to appreciate tabs here, but you have to have already established tabs as the prevailing convention for this to work.