Living with a PEP8 mafia

Coding
1

I’ve been writing python code for 17 years now. I have my own style I’m comfortable with and that I can easily read. It’s not that different from PEP8 standards - but there are significant differences. I basically follow the Maya docs standard - which is quite a bit different from PEP8. In other studios there have been suggested guidelines but people were free to write code to personal preference. I’m now being forced - yes forced - to adopt a very strict PEP8 standard in order to have my code checked in. The reviewers don’t really care about the functionality of the code, they care about too many spaces, doublequotes/singlequotes, etc. Even if I add a small method to years old code that doesn’t adhere to their new standard, I have to modify all the rest of the code that I didn’t even write.

I’m all for standards - as long as it makes it easier to read. However, the end result of this PEP8 wringer is code I find extremely unreadable. I don’t want to spend any time in that type of format - especially stuff I’m writing and will have to maintain a lot.

Anyone else had to deal with this type of behaviour before?

2

It is very common for engineers to adhere to strict coding standards that are set by engineering leads (Technical Directors, etc.). It is less common for TAs to do the same.

I don’t think it is unreasonable for standards to be enforced, and PEP8, with all its shortcomings is the standard for Python regardless of where or how it is used.

If you were an engineer, you would have hit this much earlier in your career and it wouldn’t have seemed like such a big deal since you wouldn’t have had years to cement the opinion that your coding style should be left up to you.

I’m sorry to say that from my perspective, you are better off embracing PEP8 and find more important things to stress over…

3

I see you work at EA. Even when I was at EA, it wasn’t such a big deal.

4

I don’t enforce PEP8 on my TA team, but I do encourage it generally, and will prod people to improve if stuff is extra sloppy. The engineers on my team use slightly modified PEP8 formatting for all their needs and if we were to contribute to that codebase, we would be expected to follow all their syntax guidelines. These guidelines go far beyond PEP8 and describe variable names for classes and different variable types (constants, INTs, Strings, etc.) even though Python is a typeless language. There wouldn’t be a discussion about it. We would just have to conform like everyone else…

5

(Oh, and I especially don’t care about the double versus single quotes.)

6

I led a four person TA team for a few years, and during that period we did light-weight buddy checks but did not adhere to any formal styling. When I looked at a piece of code I could see immediately who wrote it because of the difference in the way it was written (sometimes subtle and sometimes not so).

This meant that every individual was very comfortable writing code- because they all write the way they are used to writing. But it also meant that when someone was away on holiday for a couple of weeks (or leave) and you have to debug something, you’re suddenly having to work through code that feels unfamiliar. Even worse, you end up with files part written in camel case, part written in underscores etc. Everything becomes a tangled web of uncleanliness. That never happens if you’re the only TA, but as soon as your working in a team, or working in a studio that shares code with other studios it becomes a nightmare very quickly. The ‘man, next project lets re-write all this’ scenario quickly comes about.

After leading the TA team I switched over to leading a Build team for just over a year with the task of re-writing a python based build system. At first it was the same thing - we all wrote to our own style, but quickly realised how painful it was become when working heavily in shared modules etc. We collectively enforced a pep8 styling mandate - and it was painful for the first month as we were all adjusting our personal styles to match it. But it was the best decision we made!

Once we were all singing to the same style all our code looked exactly the same regardless of who wrote it. Debugging became far easier, extending and adjusting modules became a breeze - and ultimately it meant our code reviews were focused on the functionality because we were not constantly dealing with the overhead of understanding style in order to understand functionality. Therefore, indirectly our code base became more stable and robust too.

Hold in there, switching a coding style can be hard at first, but eventually it becomes natural and you will think nothing of it - but its a great thing to have a team all working to the same style and the benefits are huge - there is a reason the majority of engineering teams (both in games, software, film etc) adhere to coding standards and testing methodologies.

The thing to remember is that adopting a coding standard is about the team and production rather than the individual. Having to re-style a whole module because you need to change a variable is no bad thing - that technical debt has to be paid somewhere by someone, therefore you’re being part of the solution, relish it.

Give a month and you wont consciously be writing in the style of pep8 - it will just be happening. The studio behaviour you describe strikes me as extremely mature and well driven. Regardless of the style or standards that are adopted, having all contributors writing in the same way is a very good thing.

Crikey… that very quickly became a wall of text…

7

The fact of the matter is you work at an organization that has an agreed upon and adhered to standard. Whether you care for it or not is irrelevant. If you are unwilling or uncomfortable writing code to that standard you need to consider leaving the organization. If you stay and ignore the standard or spend all of your time railing against the standard, they’ll probably eventually ask you to leave anyway. If you view PEP8 as unreadable, imagine what your coworkers would have to adapt to if you went through the majority of the code base over time and sprinkled in functions and classes that adhere to your own standard. The entire codebase would be a disaster to comprehend and maintain.

Try it without complaint for a few weeks (honestly a week should be more than enough to adapt). If you truly hate it and cannot accept it, you need to reconsider some things. At that point staying put and expending energy on detesting PEP8 would be a waste of your time and the time of others.

8

I see one issue here, updating the code you did not write should not be needed, unless its an actual task. At least not a part of the same commit as adding/fixing functionality.

9

If it is code from the poster’s personal library that he is pasting into the company’s code to affect a fix it should be modified to match the organization’s standard. If it’s a 3rd party module/library it should be committed to an area in the tree specifically for 3rd party code and imported as necessary–but not modified to change the coding standard. Doing that would make future merges of updates from the 3rd party source practically impossible.

10

[QUOTE=Jeff Hanna;30601]If it is code from the poster’s personal library{snip}[/QUOTE]

Just a side comment, unless the code you’re copying is completely generic, or you don’t mind losing the “rights” to the code you’ve written, be very careful when doing this.

I know I’m stating the obvious…

11

> Anyone else had to deal with this type of behaviour before?

You, sir, are the one with the behaviour.

12

I view well styled code is an an act of compassion for others (and probably my future self as well)
I had to work on some C# code written by a programmer who left, and while a much more skillful and seasoned programmer than I, he tended to skip code reviews and style compliance. If he had bothered to comply, his functions would have at least been commented consistently and might have saved me days when a critical bug showed up after he moved on.

13

Lots of misreading of my original post going on here. The code I’m working with is NOT my own personal code. It is written for the facility. And the coding style is not an arbitrary style - it’s the Maya docs style. It just differs from PEP8.

But in the meantime, I have been vindicated. I listened to one of Hettinger’s talks about PEP8. He’s a python core developer. What he said is PEP8 is meant to be a suggestion, not a hammer. He said if you don’t think it makes readable code, don’t follow it. He also said a vast portion of the core python code isn’t even PEP8 compliant. He said he gets many pull requests for python code where the only thing these people have done is removed spaces, renamed variables, etc. - they have not improved the functionality of the code. He regularly rejects them. He said the PEP8 zealots are usually the fresh graduates from school where all they have to offer is to beat other people with PEP8 compliance.

That is exactly what I’m dealing with. Most of the comments for my own pull requests are from the noobs. They don’t actually make any useful suggestions on making the code function better. I had a comment from an old timer lead programmer which basically told them to leave me alone. He said I am not responsible to modify the style of the existing code when adding my new functions. And he said my code was close enough to standards to not be bothered by the other comments. Hooray for common sense.

14

[QUOTE=cgjedi;30614]What he said is PEP8 is meant to be a suggestion, not a hammer. He said if you don’t think it makes readable code, don’t follow it. [/QUOTE]

That’s the way I see it. It is a suggestion and probably the best and widest accepted guideline for formatting Python code. And I agree with the comments regarding readability and the benefits of standardization. But in a TD/TA setting counting spaces is just silly. It says nothing about the code’s quality or readability, but a lot about the organization and it’s priorities.

An organization should define how much flexibility it allows in adherence to standards, and it should review these guidelines regularly and update them as needed. It also helps to define organization wide alternatives to parts of the PEP8 developers cannot agree on. For example, formatting JSON in PEP8 is one thing where we allow a great deal of flexibility. We’re more strict with variable naming though.
My view is that unless you develop critical code that’s for the centuries, you will gain more by allowing flexibility, within agreed boundaries, and having motivated developers than pinching pennies…or spaces.

15

cgjedi - what about line width - you have to stick with 80 chars?

16

Great topic.
At my old job there was a pretty strict standard. At a high point there was around 70 Character TD’s. So we had a Code Czar to kind of oversee the framework. If a TD needed to make a new tool or script there was a command-line call that would copy over a template with all the necessary inheritance and base imports needed to work in the studio env. This template was very broiler-plate and had all the bits that people agreed should be the gold standard.

Now jump forward and I manage a small team of TD’s where each are at varying levels. I haven’t been too much of a Nazi about it but I know what people mean when having to dive into other peoples code when they are away. It does slow down ones ability to read and understand others code. I have managed to make sure everyone is using PyCharm and feel comfortable using this IDE. Given that, if everyone agrees to following some of the PEP8 standards then you could tell people to import and use some standard settings. I haven’t enforced this but as our team grows then it will become a need. The other thing that PyCharm has is a broiler-plate template you can create on New Python file. It does help keep things consistent.
-Sean

17

[QUOTE=kroopson;30620]cgjedi - what about line width - you have to stick with 80 chars?[/QUOTE]

This is one of the times where I think the spirit of the law is more important than the letter of the law. 80 characters becomes a ridiculous limit with Python code indented a few levels deep. No one is coding on a TTY terminal from the 70’s. Very few people are coding over an SSH terminal session. If you’re coding from your phone, you’d better be on vacation or something. It’s fine to say that code should be wrapped at a reasonable limit, but a hard 80 characters is pretty silly in a modern coding environment.

1 Like
18

The 80 character length is really only a hard requirement for the python standard library, even pep8 mentions that teams that can agree on a longer length should go right ahead.
Granted if you’re hoping one day to retire your library to the stdlib, then yeah, you should probably just stick with the 80 char length so you don’t have to go back and refactor for something that insane. But I’ve got a hunch most of us don’t need to worry to badly about that.

19

sound sliek they are just wanting you to adhere to pep8 as they have chosen that as their standard. It doesn’t matter what hettinger or gvr say, if your team has a standard then stick to it. its all about consistency. If you take liberties with their standard then you might as well not have one. I agree it sounds like they dont see the gorilla walk by and are blindly looking for standards compliance violations. Maybe let them get that out of their system and then see what they have to say. Sometimes i check for that first, then go through the logic. If it meets the standard then i can step beyond that and look at the code and what it does.

point is: consistency

20

just be consistent with the team, it really doesn’t matter who style the team is using as long as they are all using a similar style. If you feel it needs some changes, approach the team with the changes. Since do you know what will suck even more then dealing with pep8? Its having one person of doing their own thing and forcing the rest of the team to clean it up.