Fishtest Coding Style Guide [RFC]

[ppigazzini]

Python

Python Official Style Guide
Google Python Style Guide

Tools list:

• auto-formatter: Black
• sort imports: isort
• linting: flake8, pylint
• Python application management: pipx

Command line:

black --exclude='env|packages' .
isort --profile black --skip env --skip venv --skip worker/packages .

---

Python version

• pyenv simple Python version management for Linux/WSL
• fishtest server: Python 3.12
• fishtest worker: Python 3.6

---

CSS, HTML, Javascript

Google HTML/CSS Style Guide
Google Javasript Style Guide

Tools list:

• Node Version Manager
• auto-formatter: Prettier
• W3C markup validator

Command line:

npx prettier --write 'server/fishtest/static/{css/*.css,html/*.html,js/*.js}'

---

Shell

Google Shell Style Guide

[vondele]

I'd leave the choice to those that actually write the code. I've used black https://github.com/psf/black in some hobby projects after a recommendation.

[linrock]

following a style guide would be great. +1 to use a code formatter to set a good example in the codebase. i've used black at work before and it's not bad. worth checking out YAPF to see how they compare.

[vondele]

This would be a quick preview on a blackened version of fishtest https://github.com/vondele/fishtest/tree/fishtestBlack

[ppigazzini]

Autoformat test on this snippet from games.py:

• master

  def make_player(arg):
    return run['args'][arg].split(' ')[0]

  while games_remaining > 0:
    # Run cutechess-cli binary
    cmd = [ cutechess, '-repeat', '-rounds', str(int(games_to_play/2)), '-games', ' 2', '-tournament', 'gauntlet'] + pgnout + \
          ['-site', 'https://tests.stockfishchess.org/tests/view/' + run['_id']] + \
          ['-event', 'Batch %d: %s vs %s' % (task_id, make_player('new_tag'), make_player('base_tag'))] + \
          ['-srand', "%d" % struct.unpack("<L", os.urandom(struct.calcsize("<L")))] + \
          ['-resign', 'movecount=3', 'score=400', '-draw', 'movenumber=34',
           'movecount=8', 'score=20', '-concurrency', str(int(games_concurrency))] + pgn_cmd + \
          ['-engine', 'name=New-'+run['args']['resolved_new'][:7], 'cmd=%s' % (new_engine_name)] + new_options + ['_spsa_'] + \
          ['-engine', 'name=Base-'+run['args']['resolved_base'][:7], 'cmd=%s' % (base_engine_name)] + base_options + ['_spsa_'] + \
          ['-each', 'proto=uci', 'tc=%s' % (scaled_tc)] + nodestime_cmd + threads_cmd + book_cmd

    task_status = launch_cutechess(cmd, remote, result, spsa_tuning, games_to_play,
                                   tc_limit * games_to_play / min(games_to_play, games_concurrency))
    if not task_status.get('task_alive', False):
      break

• YAPF (pep8 and google styles)

    def make_player(arg):
        return run['args'][arg].split(' ')[0]

    while games_remaining > 0:
        # Run cutechess-cli binary
        cmd = [ cutechess, '-repeat', '-rounds', str(int(games_to_play/2)), '-games', ' 2', '-tournament', 'gauntlet'] + pgnout + \
              ['-site', 'https://tests.stockfishchess.org/tests/view/' + run['_id']] + \
              ['-event', 'Batch %d: %s vs %s' % (task_id, make_player('new_tag'), make_player('base_tag'))] + \
              ['-srand', "%d" % struct.unpack("<L", os.urandom(struct.calcsize("<L")))] + \
              ['-resign', 'movecount=3', 'score=400', '-draw', 'movenumber=34',
               'movecount=8', 'score=20', '-concurrency', str(int(games_concurrency))] + pgn_cmd + \
              ['-engine', 'name=New-'+run['args']['resolved_new'][:7], 'cmd=%s' % (new_engine_name)] + new_options + ['_spsa_'] + \
              ['-engine', 'name=Base-'+run['args']['resolved_base'][:7], 'cmd=%s' % (base_engine_name)] + base_options + ['_spsa_'] + \
              ['-each', 'proto=uci', 'tc=%s' % (scaled_tc)] + nodestime_cmd + threads_cmd + book_cmd

        task_status = launch_cutechess(
            cmd, remote, result, spsa_tuning, games_to_play,
            tc_limit * games_to_play / min(games_to_play, games_concurrency))
        if not task_status.get('task_alive', False):
            break

• Black

    def make_player(arg):
        return run["args"][arg].split(" ")[0]

    while games_remaining > 0:
        # Run cutechess-cli binary
        cmd = (
            [
                cutechess,
                "-repeat",
                "-rounds",
                str(int(games_to_play / 2)),
                "-games",
                " 2",
                "-tournament",
                "gauntlet",
            ]
            + pgnout
            + ["-site", "https://tests.stockfishchess.org/tests/view/" + run["_id"]]
            + [
                "-event",
                "Batch %d: %s vs %s"
                % (task_id, make_player("new_tag"), make_player("base_tag")),
            ]
            + ["-srand", "%d" % struct.unpack("<L", os.urandom(struct.calcsize("<L")))]
            + [
                "-resign",
                "movecount=3",
                "score=400",
                "-draw",
                "movenumber=34",
                "movecount=8",
                "score=20",
                "-concurrency",
                str(int(games_concurrency)),
            ]
            + pgn_cmd
            + [
                "-engine",
                "name=New-" + run["args"]["resolved_new"][:7],
                "cmd=%s" % (new_engine_name),
            ]
            + new_options
            + ["_spsa_"]
            + [
                "-engine",
                "name=Base-" + run["args"]["resolved_base"][:7],
                "cmd=%s" % (base_engine_name),
            ]
            + base_options
            + ["_spsa_"]
            + ["-each", "proto=uci", "tc=%s" % (scaled_tc)]
            + nodestime_cmd
            + threads_cmd
            + book_cmd
        )

        task_status = launch_cutechess(
            cmd,
            remote,
            result,
            spsa_tuning,
            games_to_play,
            tc_limit * games_to_play / min(games_to_play, games_concurrency),
        )
        if not task_status.get("task_alive", False):
            break

YAPF is a bit slower than Black on fishtest code base. YAPF takes the same time to process already formatted code, instead Black exit immediately.

time yapf -rip .

real    0m23.068s
user    1m7.609s
sys     0m2.781s

time black .

real    0m11.132s
user    1m18.313s
sys     0m3.219s

I'm not able to replicate these YAPF anomalies pointed out in some (old) comparisons:

1. idempotence: YAPF(YAPF(file.py)) != YAPF(file.py)
2. determinism: YAPF(file.py)(t_0) != YAPF(file.py)(t_1)

[dav1312]

Not sure why but this command will ignore some of the files

npx prettier --check "server/fishtest/static/{css/*.css,html/*.html,js/*[!highlight.diff.min].js}"
Checking formatting...
[warn] server\fishtest\static\css\application.css
[warn] server\fishtest\static\js\spsa_new.js
[warn] Code style issues found in 2 files. Forgot to run Prettier?

npx prettier --check "server/fishtest/static/{css/*.css,js/*.js}"
Checking formatting...
[warn] server\fishtest\static\css\application.css
[warn] server\fishtest\static\js\sprt.js
[warn] server\fishtest\static\js\spsa_new.js
[warn] Code style issues found in 3 files. Forgot to run Prettier?

[peregrineshahin]

also, I don't see a .mak in the formatting command, which formatting there should be done also :/

[dav1312]

I don't see a .mak in the formatting command

I don't think prettier supports mako templates, either format the scripts individually (copy and paste them here https://prettier.io/playground/) or move them to js files so they can be formatted with the command automatically

[ppigazzini]

I formatted the mako templates by hand, leaving unformatted the <script> tags. BTW the mako documentation uses different code formatting, so I randomly picked one type.