Hi!

Krita doesn't have any formal style guides yet. I added T6588 to address this, although it's not likely to get much further unless we sit down and figure out what's important and what works for Krita.

PEP8 is great and should definitely serve as a base. Rule E501 (79 characters per line limitation) is a good rule of thumb but something I've found can reduce code readability, make debugging harder, and encourage bad coding practices in some cases. For this reason I often end up adding it to the autoformatting and compliance test ignore list, although changing the character limit could work too.

I've already added a flake8 config and instructions to run it, and my plan was to squash the flake8 warnings bit by bit and have a bit of an eye on the code, and bring up any issues worth discussing as I find them, such as right now the line lengths (and keep notes for documenting the stuff). From what I've seen so far from the existing plugins and the ones I've written myself, I don't expect much else that's going to be controversial, or an issue with Krita plugins specifically, regarding PEP8.

But if people would rather discuss this up front before I proceed, that's fine by me, too. :)

PEP8 is a great base, and except for the line length generally pretty much used as is. If anything, one could go further and add to it for Krita specifically, but we don't have to do that all at once, as far as I'm concerned.

Re line length: I don't personally like lines that go on forever, so I'd be in favour the 119 char limit, if not the original 79. I could make a patch with the 119 limit applied, for comparison. Krita doesn't have super long API names anyway. But whatever people are most comfortable with. EDIT: Of course, even with a character limit, you can always exclude individual lines from code check with the "# NOQA" directive, should a long line be really necessary for readability.

In D18708#405077, @rbreu wrote:

I've already added a flake8 config and instructions to run it, and my plan was to squash the flake8 warnings bit by bit and have a bit of an eye on the code, and bring up any issues worth discussing as I find them, such as right now the line lengths (and keep notes for documenting the stuff).

I like the initiative! I started fixing scripts a while back, but then work consumed all my time and energy so I had to pause it. Glad someone else is looking at it. :)

My plan was to figure out which PEP8 warnings should be fixed and which ones should be ignored or tweaked, then look at adding a compliance test step to the Jenkins build job for plugins packaged with Krita.

I expect I won't be very active for another couple of months, but I don't mind discussing the topic, either here or on IRC.

From what I've seen so far from the existing plugins and the ones I've written myself, I don't expect much else that's going to be controversial, or an issue with Krita plugins specifically, regarding PEP8.

I don't expect much of the current code to be problematic either. The only one I've found to cause problems in larger projects is the line length limitation (E501), and only for a minority of the code. That said, I like to divide style guides into two groups: Rules that should be followed, and if possible enforced, and guidelines that you should consider but are free to ignore if they don't make sense for you.

Since the line length limitation only makes sense in some contexts, I often exclude it from automated tests.

Other than that we're likely going to have to disable a number of PEP8/flake8 naming convention rules such as N802. Reason being that Krita's C++ style guidelines have different naming conventions, and if a C++ function is exposed and overridden in Python, flake8 won't be able to tell that it's an overridden function and might complain.

But if people would rather discuss this up front before I proceed, that's fine by me, too. :)

I'm fine with this patch as-is. Breaking comments into multiple lines is an improvement, although breaking a function call with a single parameter into multiple lines is IMO unnecessary. In these cases it's such a minor thing that I don't mind, but with the 79 character limit there are bound to be cases in the future where it doesn't make any sense to be that strict.

PEP8 is a great base, and except for the line length generally pretty much used as is. If anything, one could go further and add to it for Krita specifically, but we don't have to do that all at once, as far as I'm concerned.

Agreed. I would suggest that we initially ignore rules that we suspect are problematic at first, then tighten the restrictions if needed. We could also do it the other way around, and start out with full PEP8 compliance, then loosen the rules as needed. However, from personal experience I've found that people are more likely to make detrimental changes to their code in the name of compliance, rather than question the rules. So we might inadvertently encourage bad coding practices without knowing it.

Re line length: I don't personally like lines that go on forever, so I'd be in favour the 119 char limit, if not the original 79. I could make a patch with the 119 limit applied, for comparison. Krita doesn't have super long API names anyway. But whatever people are most comfortable with. EDIT: Of course, even with a character limit, you can always exclude individual lines from code check with the "# NOQA" directive, should a long line be really necessary for readability.

Likewise, although I definitely think 79 characters is way too restrictive; Python syntax means you frequently lose 12 or more characters to indentation. Coupled with long function names, "namespaces", and nestled objects that you frequently run into when interacting with C++ and Qt in particular, 79 characters is not nearly enough.

119 characters could be a good compromise. When Qt is involved I can easily see 100 character lines being common, depending on how imports are done.

Additionally, any line length limitation is going to discourage verbose inline comments, but maybe that's a good thing.