# Proposal create project coding standard for tryton

**URL:** https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295
**Category:** Organisation
**Tags:** project
**Created:** [February 5, 2020, 4:36pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295 "2020-02-05T16:36:32Z")
**Posts on this page:** 20
**Page:** 1

<div class="post-metadata">

### Author: ![dvdmgl](https://discuss-cdn.tryton.org/letter_avatar_proxy/v4/letter/d/8dc957/32.png) [@dvdmgl](https://discuss.tryton.org/u/dvdmgl)
#### Post date: [February 5, 2020, 4:36pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/1 "2020-02-05T16:36:32Z")

</div>

# Proposal to create a project coding standard

use of [pylint](http://pylint.pycqa.org/en/latest/), automatic code formatting with [black](https://github.com/psf/black), start adding [type hints](https://docs.python.org/3.5/library/typing.html) to core libs.

### Props

- improves code consistence and readability
- avoids annoying editor warnings, “_save time and mental energy for more important matters_”
- avoids _nagging about formatting_
- pylint is a tools that helps preventing certain type of bugs bugs, suggesting improvements
- helps current developers and to board in new developers

### Cons

- applying black, pylint will create possible merge and rebase issues for existing banches
- discussion about allowed variables names pylint and pep8 vs internal coding style example `FiscalYear = pool.get('account.fiscalyear')` vs `fiscal_year = pool.get('account.fiscalyear')`

### Tasks

- discus a coding style to be shared across projects
- publish aproved coding style at Tryton Documentation
- create a `pylintrc` with the approved coding style
- discus a time to apply black across all projects
- add a `# pylint: disable=` at the beginning of each python file with the rules that are not implemented, to ease development and progressive implementation of the coding style
- implement hooks to ensure that pylint and black are enforced at commit

### Future

- as of python 3.5 was introduced [type hints](https://docs.python.org/3.5/library/typing.html) would be helpful to introduce on core libs

---

<div class="post-metadata">

### Author: ![dabada83](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/dabada83/32/67_2.png) [@dabada83](https://discuss.tryton.org/u/dabada83)
#### Post date: [February 5, 2020, 5:00pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/2 "2020-02-05T17:00:32Z")

</div>

You can check coding guidelines on: [https://www.tryton.org/develop#coding-guidelines](https://www.tryton.org/develop#coding-guidelines)

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 5, 2020, 5:11pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/3 "2020-02-05T17:11:53Z")

</div>

> [@dvdmgl](#):
>
> improves code consistence and readability

This points have very few to do with the coding style.  
The key points are: good naming and proper design. And they are not and cannot be managed by tools.

> [@dvdmgl](#):
>
> avoids annoying editor warnings,

Well that depends on everyone editors.

> [@dvdmgl](#):
>
> avoids _nagging about formatting_

As I’m doing most of the reviewing, I have very little comment about format. And most of the time reviewbot has already commented.

> [@dvdmgl](#):
>
> “ _save time and mental energy for more important matters_ ”

Indeed I find that the formatting comes automatically out of a well though design. And usually bad code has bad design (even if it was linted or whatever).

> [@dvdmgl](#):
>
> pylint is a tools that helps preventing certain type of bugs bugs, suggesting improvements

We have already flake8 run by reviewbot. I do not think we must change but [improve it](https://bugs.tryton.org/issue7213).

> [@dvdmgl](#):
>
> helps current developers and to board in new developers

I do not see how if they have to setup more tools in order to start.

> [@dvdmgl](#):
>
> applying black, pylint will create possible merge and rebase issues for existing banches

Indeed it is more about backport of fixes. But it would be a major problem.

> [@dvdmgl](#):
>
> as of python 3.5 was introduced [type hints](https://docs.python.org/3.5/library/typing.html) would be helpful to introduce on core libs

I already though about that but I could not find a way to deal with dynamic class generation Tryton has.  
Maybe with a hook that create stub files automatically out of all the modules.  
But anyway, this is off topic about code style.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 5, 2020, 5:53pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/4 "2020-02-05T17:53:40Z")

</div>

> [@ced](#):
>
> > applying black, pylint will create possible merge and rebase issues for existing banches
> 
> Indeed it is more about backport of fixes. But it would be a major problem.

Also for now our style was designed with back-port of patches in mind. So it tries to minimize the number of line modified (e.g. fixed indentation depending on number of opening brackets).  
But an automated formatting does not do that, it may decide to use a different style because some limits are reached (I think about list with 1 item per line for example).

---

<div class="post-metadata">

### Author: ![JCavallo](https://discuss-cdn.tryton.org/letter_avatar_proxy/v4/letter/j/48db29/32.png) [@JCavallo](https://discuss.tryton.org/u/JCavallo)
#### Post date: [February 4, 2022, 8:38am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/5 "2022-02-04T08:38:44Z")

</div>

Hello,

Since [black](https://github.com/psf/black) is now officialy stable, I would like to re-open this discussion.

Just to be clear, I really do not like the black coding style (and I really like the current Tryton style). However, as the main reviewer in a company with dozens of developpers, eliminating the style issues from reviews would be invaluble.

> [@ced](#):
>
> > applying black, pylint will create possible merge and rebase issues for existing banches
> 
> Indeed it is more about backport of fixes. But it would be a major problem.

Actually, since it is “just” formatting, wouldn’t it be possible to just apply it on all maintained branches?

> [@ced](#):
>
> > [@](#):
> >
> > helps current developers and to board in new developers
> 
> I do not see how if they have to setup more tools in order to start.

Since black is the (AFAIK) only widely used python formatter, most python users will already know about it.

> [@ced](#):
>
> > [@](#):
> >
> > avoids annoying editor warnings,
> 
> Well that depends on everyone editors.

Ignoring E127 / E128 sometimes lets slip formattings that should not be

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 4, 2022, 8:49am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/6 "2022-02-04T08:49:01Z")

</div>

> [@JCavallo](#):
>
> Just to be clear, I really do not like the black coding style (and I really like the current Tryton style).

Me too and even more I find it makes reading more difficult.  
So it is the end of discussion?

> [@JCavallo](#):
>
> Actually, since it is “just” formatting, wouldn’t it be possible to just apply it on all maintained branches?

No because the problem is applying back-port.

> [@JCavallo](#):
>
> Since black is the (AFAIK) only widely used python formatter, most python users will already know about it.

I personally do not like where the current Python community is going with formatting, typing etc. They are removing the simplicity and beauty of Python and make it looks like other languages.

> [@JCavallo](#):
>
> Ignoring E127 / E128 sometimes lets slip formattings that should not be

The main reason we do not follow that is it makes backport really harder.  
Indeed it will be good if we could tune flake8 to enforce line break at opening parenthesis.

---

<div class="post-metadata">

### Author: ![dave](https://discuss-cdn.tryton.org/letter_avatar_proxy/v4/letter/d/ee7513/32.png) [@dave](https://discuss.tryton.org/u/dave)
#### Post date: [February 4, 2022, 10:14am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/7 "2022-02-04T10:14:44Z")

</div>

> [@JCavallo](#):
>
> Just to be clear, I really do not like the black coding style (and I really like the current Tryton style).

I fully agree with this.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 7, 2022, 12:48am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/8 "2022-02-07T00:48:24Z")

</div>

I just made a test with `black -l 79 -S`. It is not that bad but the having closing bracket on a single line is quite annoying lose of space. I guess it may be good to also test [yapf](https://pypi.org/project/yapf) which seems to have more configuration options (and which may be adapted to our style).

But at the end I think this can be applied only if we have a way to ignore the commit from the blame command. I remember seeing a talk from Octobus guys showing such tool but I can not find it back (if I remember correctly, it was an experimental feature).

---

<div class="post-metadata">

### Author: ![JCavallo](https://discuss-cdn.tryton.org/letter_avatar_proxy/v4/letter/j/48db29/32.png) [@JCavallo](https://discuss.tryton.org/u/JCavallo)
#### Post date: [February 9, 2022, 9:01am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/9 "2022-02-09T09:01:14Z")

</div>

Here is the [rationale](https://github.com/django/deps/blob/main/accepted/0008-black.rst) behind Django’s adoption. All may not be relevant, but I think it is interesting, since they also had the same questions

---

<div class="post-metadata">

### Author: ![nicoe](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/nicoe/32/2880_2.png) [@nicoe](https://discuss.tryton.org/u/nicoe)
#### Post date: [February 9, 2022, 9:34am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/10 "2022-02-09T09:34:12Z")

</div>

> [@ced](#):
>
> But at the end I think this can be applied only if we have a way to ignore the commit from the blame command. I remember seeing a talk from Octobus guys showing such tool but I can not find it back (if I remember correctly, it was an experimental feature).

In the django discussion they talk about `darker` which applies black only on the region that change.

> **[darker](https://pypi.org/project/darker/)**
>
> Apply Black formatting only in regions changed since last commit

I am puzzled on this idea, it solves the issue but it’s strange to only black part of the code.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 9, 2022, 9:53am UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/11 "2022-02-09T09:53:53Z")

</div>

> [@nicoe](#):
>
> In the django discussion they talk about `darker` which applies black only on the region that change.

It will need to be adapted for mercurial but it does not seem to complicated as mercurial can output git-like diff.

> [@nicoe](#):
>
> I am puzzled on this idea, it solves the issue but it’s strange to only black part of the code.

Indeed I like the idea for a smooth transition. We can expect that after 1-2 years, most of the code will have been formatted.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 10, 2022, 4:38pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/12 "2022-02-10T16:38:52Z")

</div>

> [@ced](#):
>
> But at the end I think this can be applied only if we have a way to ignore the commit from the blame command.

Indeed the `hg annotate` command has a `--skip (EXPERIMENTAL)` option.  
[Mozilla is using it](https://bugzilla.mozilla.org/show_bug.cgi?id=1508002) with a custom command line that get revision from a file.  
I found also [this interesting way](https://stackoverflow.com/questions/20287201/is-there-a-way-to-ignore-a-commit-in-hg-blame#comment105659562_55194672) using pattern matching in commit message. It is probably slower than having a file with revision.  
Any way we should probably see for the [Heptapod migration](https://discuss.tryton.org/t/migration-of-development-to-heptapod/4971/13) to see what can be supported.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 10, 2022, 11:18pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/13 "2022-02-10T23:18:21Z")

</div>

For Javascript, we could use [Prettier](https://prettier.io/) but it does not support mixing quote like we do in Python between “keyword” and “literal”.

---

<div class="post-metadata">

### Author: ![acaubet](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/acaubet/32/1493_2.png) [@acaubet](https://discuss.tryton.org/u/acaubet)
#### Post date: [February 17, 2022, 6:30pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/14 "2022-02-17T18:30:42Z")

</div>

I just found [ssort](https://github.com/bwhmather/ssort), could serve to maintain a good order in the methods.

---

<div class="post-metadata">

### Author: ![nicoe](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/nicoe/32/2880_2.png) [@nicoe](https://discuss.tryton.org/u/nicoe)
#### Post date: [February 17, 2022, 6:54pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/15 "2022-02-17T18:54:17Z")

</div>

I am reviewing a lot of module code currently and so I see sometimes some pretty complicated domain.

Here’s what black does:

```auto
    tax = fields.Many2One(
        "account.tax",
        "Tax",
        domain=[
            (
                "company",
                "=",
                Eval("_parent_line", {})
                .get("_parent_move", {})
                .get("company", -1),
            ),
        ],
        depends={"line"},
    )

```

while my “ideal” code would look like this:

```auto
    tax = fields.Many2One(
        "account.tax", "Tax",
        domain=[
            ("company", "=",
                Eval("_parent_line", {})
                .get("_parent_move", {})
                .get("company", -1)),
        ],
        depends={"line"})

```

I do get that it’s impossible for black to understand what I want as it’s a matter of taste, nevertheless I find its version quite ugly.

Moreover in this example there is only on leaf in the domain, I wonder what happens when there is two / three complicated leaves.

---

<div class="post-metadata">

### Author: ![ced](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/ced/32/1237_2.png) [@ced](https://discuss.tryton.org/u/ced)
#### Post date: [February 17, 2022, 10:03pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/16 "2022-02-17T22:03:12Z")

</div>

> [@nicoe](#):
>
> Moreover in this example there is only on leaf in the domain, I wonder what happens when there is two / three complicated leaves.

It will clearly separate each clause which may be annoying from where we come but maybe it is not that bad for readability.

As counter argument, I think this kind of domain should not be written. It will be better to have a `company` Function field because it makes the domain working inside the One2Many’s but also outside. And cherry on the cake, it allows to right the proper multi-company rule.

> [@acaubet](#):
>
> I just found [ssort](https://github.com/bwhmather/ssort), could serve to maintain a good order in the methods.

What will be the output of common Tryton’s class?

---

<div class="post-metadata">

### Author: ![acaubet](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/acaubet/32/1493_2.png) [@acaubet](https://discuss.tryton.org/u/acaubet)
#### Post date: [February 21, 2022, 2:22pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/17 "2022-02-21T14:22:00Z")

</div>

> [@ced](#):
>
> What will be the output of common Tryton’s class?

After some try the output it is not very helpfull (to much oppinionated maybe). When I proposed I was thinking to use it in a way to avoid [such reviews](https://codereview.tryton.org/337731002/diff/381471002/modules/account_statement/statement.py#newcode965):

> We usually extend ORM method at the end.

---

<div class="post-metadata">

### Author: ![nicoe](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/nicoe/32/2880_2.png) [@nicoe](https://discuss.tryton.org/u/nicoe)
#### Post date: [February 21, 2022, 3:07pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/18 "2022-02-21T15:07:16Z")

</div>

> [@acaubet](#):
>
> After some try the output it is not very helpfull (to much oppinionated maybe). When I proposed I was thinking to use it in a way to avoid [such reviews](https://codereview.tryton.org/337731002/diff/381471002/modules/account_statement/statement.py#newcode965):
> 
> > We usually extend ORM method at the end.

Yes that’s something that always bothers me.

My way of defining the methods would be:

1. ` __new__ ` and ` __init__ ` (but we should never use them in Model)
2. ` __register__ ` then ` __setup__ `
3. `default_` methods
4. `on_change_` and `on_change_with_` methods
5. function getters and setters
6. ORM methods
7. `view_attributes`
8. workflow methods (and the helper methods used by those before them).
9. other methods

Some choices are arbitrary, others are pretty obvious and some are good™ but I can’t explain why. And when extending I would respect the same order.

But of course, it’s difficult to always respect this or even remembering to do it. Hence I think that ssort could be a good idea (but I am afraid it’s even more difficult to do than format code because there is a lot of semantic there).

---

<div class="post-metadata">

### Author: ![JCavallo](https://discuss-cdn.tryton.org/letter_avatar_proxy/v4/letter/j/48db29/32.png) [@JCavallo](https://discuss.tryton.org/u/JCavallo)
#### Post date: [March 14, 2022, 3:10pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/19 "2022-03-14T15:10:06Z")

</div>

> [@nicoe](#):
>
> - ` __new__ ` and ` __init__ ` (but we should never use them in Model)
> - ` __register__ ` then ` __setup__ `
> - `default_` methods
> - `on_change_` and `on_change_with_` methods
> - function getters and setters
> - ORM methods
> - `view_attributes`
> - workflow methods (and the helper methods used by those before them).
> - other methods

I globally agree on this order, just wondering why ORM methods are this “low”.

I would say that since they may change things significantly **and** globally, they should rather be at the top (right after the ` __register__ ` / ` __setup__ `).

Also, I would love to find a way to visually group methods related to a single field (default / on\_change / domain / order / getter / searcher / setter) together, but no great idea so far… Best solution was to use manual vim folds, but that is not universal enough.

---

<div class="post-metadata">

### Author: ![nicoe](https://discuss-cdn.tryton.org/user_avatar/discuss.tryton.org/nicoe/32/2880_2.png) [@nicoe](https://discuss.tryton.org/u/nicoe)
#### Post date: [March 15, 2022, 1:09pm UTC](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295/20 "2022-03-15T13:09:33Z")

</div>

> [@JCavallo](#):
>
> I globally agree on this order, just wondering why ORM methods are this “low”.

Because it’s first the method / fields that define the model and then the methods that define its behaviour. And overriding the ORM m

But I don’t think it would be doable to define a standard that would work in all the situations. It’s more guidelines to take into account.

[Next page](https://discuss.tryton.org/t/proposal-create-project-coding-standard-for-tryton/2295.md?page=2)
