In this series, we're exploring how real-world projects use the Google Python Style Guide a.k.a GPSG. Let's start with the most obvious candidate: the Sourcery repository itself.

At Sourcery, we work with a monorepo that contains the vast majority of our code:

• the CLI
• the common basis for our VSCode and PyCharm plugins
• the built-in rules
• some custom rules, including the GPSG
• the Sourcery refactoring engine

We've been using various code quality tools since the very beginning:

• pylint
• mypy
• Sourcery core (obviously 😉)
• several Sourcery custom rules

Applying the GPSG felt like a logical next step to make our codebase even more maintainable and more fun to work with.

Although it's a huge and versatile repo, our coding conventions are pretty much the same everywhere. If you also work with a monorepo, you might need more "local" rules for various parts of your codebase. For example, if different parts of your codebase use different frameworks. In that case, you can use the paths property of the rules to restrict their scope.

The First Review

A first review with the GPSG gave a great picture of the current status of our repo.

1. We copied the complete rule set of the GPSG into the .sourcery.yaml file of our repo below our existing rules.

Tip: If your repo doesn't have a .sourcery.yaml file, generate it with sourcery init. This will add some sensible defaults, e.g. ignoring directories like .git and virtual environments.

2. We ran a Sourcery review in the CLI:

sourcery review .

We found:

• lots of missing docstrings
• some hundred missing type annotations
• ca. 70 variables with a single-character name (mainly in tests)
• ca. 70 names with a suffix indicating the type
• a small number of violations for several other rules

From this, we already learned several things:

• In many topics, we agree with the GPSG and tend to write compliant code.
• Despite of this agreement, we've introduced several violations over the years. In a huge codebase, even if you break the rules rarely, it means several dozen cases. Some of them are probably valid exceptions where it makes sense to override the rule; others are clearly technical debt.
• The major topic where we didn't follow the GPSG is docstrings.

Goals

We quickly agreed on the following goals:

• Set high standards for any new code added to the repo.
• Focus on the areas that aren't covered by tools already in use.
• Do not require fixing legacy issues.

We generally recommend taking an incremental approach regarding legacy issues. It's essential to be aware of them, but trying to clean them up at once isn't a good idea.

A tempting but dangerous approach is to include such fixes in a random PR that was supposed to add a new feature or fix a bug. Those "quick adjustments" can easily distract both the developers and the reviewers from the actual scope of the PR. Of course, if you detect some issues that are very important or straightforward to fix, you're encouraged to fix them. But do it in a separate PR that is dedicated to cleaning up this tech debt instead of mixing it with other work.

Implementation

Based on these goals, we chose a fairly common implementation approach:

• Add the majority of the GPSG rules to our rule set.
• Enforce the rules only on newly added and changed code.
• Fix some legacy issues in occasional cleanup PRs.

To achieve this, we have the following setup:

• a pre-commit hook
• a GitHub Action in our CI

Both use the --diff option of the Sourcery CLI to check only the currently changed code.

Additionally, every team member has the Sourcery extension installed in their IDE (VSCode, PyCharm, Vim). This isn't strictly necessary, but very useful for detecting possible violations early.

Discussing Our Code Conventions

We set up a team meeting to discuss which GPSG rules we wanted to follow in our repo. We scheduled it for 1 hour, but it turned out to be quicker.

Again, we copied the whole GPSG rule set into our .sourcery.yaml file below our existing custom rules. Then we went through them group by group or, in some cases, rule by rule.

Import Rules

This was an easy one. The Import Guidelines section of the GPSG contains widely accepted best practices, and we are happy to follow them. Avoiding wildcard and relative imports makes the code easier to grasp both for humans and static analysis tools. Obviously, we care about both here. ;-)

Naming Conventions

Establishing our naming conventions was less straightforward. The 3.16 Naming Rules section of the GPSG is quite versatile. You'll probably decide rule by rule whether to follow it.

We quickly agreed that we didn't want single-character names, so we kept those 2 rules: one for variable names, one for function names. We also added all the rules that enforce the PEP8 naming conventions:

• snake case for functions, arguments, and variables
• camel case for classes

However, we had an interesting discussion about the name-type-suffix rule. We knew that our codebase contained several such suffixes but weren't sure how many and where exactly. So, we ran another review, this time only for this single rule:

sourcery review --include name-type-suffix .

We found that the 72 names with type suffix appear at various parts of the codebase. We randomly looked at some of them and the names looked sensible. So, we decided to skip this rule.

This is a complex rule where you have several options to take a less extreme approach than our team. 😀

• If you have only a few variables with a meaningful type suffix, the easiest solution is probably to add a # sourcery skip comment to them.
• If you have many variables where the suffix _str makes sense, you can keep the rule and remove the or name.ends_with("_str") part of its condition.

No Long Functions

We had a quick agreement here. Although our code base has a few functions that are longer than 40 lines, we regard those as technical debt.

Function length has a strong correlation with other code quality metrics:

• Longer functions tend to be more complex.
• Longer functions tend to contain more local variables posing a considerable demand on the reader's short term memory. This increases the chance of overseeing something.

We encourage you to keep this rule, even if it feels strict for your current codebase. Consider adding a # sourcery skip comment to the existing too long functions. And if it feels to obvious, you can experiment with setting the limit lower than 40. 😉

Docstrings

We had the most prolonged discussion about the docstrings rules. This is not a surprise, as there are valid arguments in both directions in the broader community as well. :-)

• The GPSG itself says that the vast majority of functions need a docstring. The only exceptions are very short, obvious, and externally not visible functions.
• Others argue for less emphasis on completion. Instead, focus on documenting the crucial and tricky parts of the system. (Like Martin Fowler's thought-provoking article about The Almighty Thud)

We decided to give it a try and included all the docstring rules from the GPSG in our config. We added the rules for modules and classes as they were. However, we tweaked the docstring rule for functions:

• Changed the limit for short functions from 5 to 10 lines.
• We excluded functions starting with the prefix enter_ or leave_. (Those are frequent constructs in our Sourcery core engine.)

condition: not ((f.starts_with("_") and body.statement_count() < 11) or pattern.in_function_scope())
  and not f.starts_with("enter") and not f.starts_with("leave")

Adding the docstring rules also means that we've also started to add docstrings to already existing functions and modules whenever we touch them. I'm curious whether they will make our life easier.

Type Hints

Adapting the type annotation rules really depends on your codebase and on your preference: How many type hints do you already have and do you want to use them more? (For a great discussion about the advantages and disadvantages of type hints, check out this interview with Luciano Ramalho at The Real Python Podcast.)

If your code doesn't include many type hints and you want to gradually increase their number, the GPSG might be a great choice because of its flexibility. For example, you can start with only a subset of the rules or apply them only for public functions.

Our team was in a different position: We already use mypy for enforcing type hints and weren't sure whether we wanted to add more checks from another tool about the same topic. So, after some back and forth, we decided to skip these rules for now.

Living Style Guide

Generally, we regard the outcome of this meeting as version 1 of our coding conventions. It isn't a law set in stone but a yaml file in our repo. Everybody is encouraged to contribute to it via our usual pull request process.

Having these rules written down and spelled out as code allows you to have better and more explicit discussions around them. (Instead of revisiting the same topic again and again at code reviews.) This is valuable both for best practices and for cases where you choose between equally good alternatives for consistency.

Finding the tailored conventions for your codebase is an iterative process. Evaluate continuously whether a specific rule helps you write more robust and more maintainable code. Feel free to ditch it if it adds too much overhead.

Have you tried out the GPSG? Have you defined some custom rules for your codebase? Reach out at hello@sourcery.ai We'd be happy to chat about your (written and unwritten) code conventions.