PEP8 suggestion

[technic]

Dear all,
enigma2 python code base is good in general, but code style is sometimes random.
What do you think about adopting python linter to format code according to pep8,
and enforce these check in the CI?
The drawbacks for this is that git history would be messed up with this huge commit, and doing merges between other forks could become problematic.
I will appreciate answer from maintainers, who have bigger picture of the project.

[WanWizard]

Personally, I support anything that is an improvement, and if that means biting the occasional bullet, so be it.

@eriksl @littlesat @betacentauri @athoik @technl @Huevos @AbuBaniaz ?

[athoik]

Sure PEP8 is fine, we can follow it with one exception only, tabs instead of spaces.

[WanWizard]

+1 👍

[betacentauri]

We should only find a time slot when this has not much side effects. Eg I would love to merge some of my python3 commits(which also work with python2) before pep8 changes are done. Otherwise I guess I will get many merging problems.

[Huevos]

I am 100% against this suggestion. The enigma2 community is already fragmented without people dreaming up ways to make sharing code bug prone to impossible. And the idea of doing this in one huge commits is madness. Please leave this idea for new code only so we can continue to share code between images. This suggestion brings no functional benefit whatsoever. It just makes others lives more difficult. Time that could be better spent writing code rather than just fiddling.

[technic]

Sure PEP8 is fine, we can follow it with one exception only, tabs instead of spaces.

To my experience, linters don't always work well with tab + spaces mixture. I tried to use autpep8 for my plugin code base where I use tab for indentation and spaces for alignment, and I always got "Continuation line over-indented" error in this case.

[WanWizard]

@Huevos, I don't understand why standardising code formatting will make "sharing code bug prone to impossible". Can you explain what you mean by this?

[Huevos]

Because the pep8 commit well be impossible to merge into other images for obvious reasons. That means all subsequent code sharing will have to be done manually, which is highly error prone.

Keep the pep8 for new code. Don't break code sharing. Also it will discourage people like me that have pushed a lot of code back to PLi over the last 10 years.

[WanWizard]

You just highlighted the biggest drawback or downside of forking. For the rest of the life of the fork, you'll have to merge upstream changes, and manually fix merge conflicts. This is just another upstream change.

You're basically saying the existing code can't be corrected/standardized because you don't want to deal with the merge conflicts that could result from that?

There is imho no reason to do it all in one big commit, it can easily be done on a per file or per plugin basis. But that doesn't address your merge conflicts.

[technic]

The whole zoo of enigma2 forks is a bit depressing thing. I understand, that DMM closed sources and we have to maintain open-source fork here. But why are there so many other repositories, which even not marked as forks in the github interface? :( And after spending some time for developing plugin those forks make me problem, because they often break API of common core classes.

Keep the pep8 for new code. Don't break code sharing. Also it will discourage people like me that have pushed a lot of code back to PLi over the last 10 years.

Anyway we may check this out: https://pep8speaks.com/.
And pycodestyle also has --diff option for this purpose.
We can ignore tabs and double line breaks, this will leave us just with slight changes in list and dict definitions and other spaces issues in the middle of the lines, this shouldn't be big diff.

[Huevos]

@WanWizard I agree about forking but that is our start point.

Problem with a 'pep8 clean up' is it affects the complete file. That means merging it from one image to another for identicality would be impossible to do automatically. Doing it manually will introduce bugs for sure.

And anyway who says pep8 is correct? It is just a recommended coding style.

What my main objection is, is that for no functional benefit someone is going to feed all the code to an automated pep8 clean up tool and in seconds they are going to create hundreds of hours of work for the people that create real functional code that brings new and interesting features to the code base and image.

[WanWizard]

Afaik with git (and I am by no means an expert) you can merge such a commit, reject all conflcts, and merge the remaining (and now empty) commit. That is a one-time action.

That does however create a challenge contributing back upstream, as there will now be a difference in coding style (spaces vs tabs for example). So it means we have to be careful when accepting PR's.

I have a PHP background, and I push for standards in every project I've worked on, it will always pay off when it's time to do maintenance or functional changes. I don't have enough knowledge of Python and PEP8 to judge whether or not the benefits outweigh the (imho short-term) pain.

[betacentauri]

If it’s an output of a tool, just use the tool with the same command line options on the other image. Effect should be the same.

[technic]

I am not a git expert as well, but if we provide a simple command line to run linter and do the changes, then every fork can easily do the same command before merge. And then git should be able to handle this situation, showing only real changes.

[Huevos]

@WanWizard main difference between python and PHP is PHP uses the ";" as the separator and curly braces around the clauses and loops. Python uses indenting instead. So python is already cleaner whereas PHP the whole script could be written on one line. The PHP interpreter can read it but it is a right mess for a coder to read.

[WanWizard]

@Huevos I know that, I'm not a complete n00b 😀

PEP8 is about reading as well, it's it main aim, to improve readability. The biggest challenge will be naming conventions and programming recommendations, as the codebase is a complete mess when it comes to encapsulation, there are globals (variables and functions) all over the place, and tight coupling is the norm. You pull one string here, and elsewhere something tumbles over... 😭

[AbuBaniaz]

I suppose most of the new coders will be using the new style. So it will be advantageous to be in a situation where the newer blood can contribute instead of having to re-learn what they know and apply it in a different format.

If there is agreement to convert.....

IMO, an automated process would introduce problems. Should there be a wish to do it in one go, a new branch should be used, perhaps devleop-pep8 and see how merging/picking from develop works in practice.

A possible approach may be to change files one at a time when they get modified. One commit pep8 conversion, next commit for actual code change. A bit laborious, but ensures that changes are done slowly and easier to fix. Yes, they will come a stage where all remaining files should be converted, but I suppose the previous incremental changes would get the old hands used to the newer style.

[littlesat]

I suggest we should strive pep-8 and when we change some code also strive to change it where you need to perform changes and not the whole bunch of files... so 🍒 picking and merging will not be more hard than wished...

[Huevos]

Thank you.

[AbuBaniaz]

Other images have added @persianpros pep bots. maybe these can be added to PLI once 9.0 is out.