Inconsistencies fix

[Dark-Rock]

Pull Request

Related issue

Fixes #563

What does this PR do?

• Fixed the inconsistencies using black throughout the project. This changed the single quote to double quote (Human readibility reasons), fixed the parenthesis anomaly and improved readibility of the code base.

PR checklist

Please check if your PR fulfills the following requirements:

• Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
• Have you read the contributing guidelines?
• Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

[alallema]

Hello @Dark-Rock,
Thank you very much for contributing to Meilisearch ❤️.
However, I am not available on the weekend, but I will be back on Monday 😊.

PS: This message was sent automatically!

[sanders41]

First, if this PR is accepted or not is up to @alallema and not me. With that I wouldn't work on anything I mention here until she decides if she want to accept it or not.

Black should be added as a dev dependency and also a CI check, otherwise things will get out of sync again. My suggestion for the pyproject.toml would be:

[tool.black]
line-length = 100
include = '\.pyi?$'
exclude = '''
/(
    \.egg
  | \.git
  | \.hg
  | \.mypy_cache
  | \.nox
  | \.tox
  | \.venv
  | \venv
  | _build
  | buck-out
  | build
  | dist
  | setup.py
)/
'''

The default line length for black is 88 so the 100 is just my personal preference, @alallema if you decide to accept this at all line length is totally your call and can be anything you want. The other settings are default black settings (and black purposefully has almost no settings options).

Then in tests.yml add a black check:

black:
    name: black
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - name: Set up Python 3.9
        uses: actions/setup-python@v2
        with:
          python-version: 3.9
      - name: Install pipenv
        uses: dschep/install-pipenv-action@v1
      - name: Install dependencies
        run: pipenv install --dev
      - name: black
        run: pipenv run black meilisearch tests setup.py --check

Either as part of this PR or as a separate PR I would also add isort. For this, add isort as a dev dependency with the following in pyproject.toml matching line_length to whatever gets decided for black

[tool.isort]
profile = "black"
line_length = 100
src_paths = ["meilisearch", "tests", "setup.py"]

And the following CI check

isort:
    name: isort
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      - name: Set up Python 3.9
        uses: actions/setup-python@v2
        with:
          python-version: 3.9
      - name: Install pipenv
        uses: dschep/install-pipenv-action@v1
      - name: Install dependencies
        run: pipenv install --dev
      - name: isort
        run: pipenv run isort meilisearch tests setup.py --check-only

mypy and pylint setting could also be moved to the pyproject.toml, but that is an easy follow-up PR.

@alallema for some context, this will make your life easier in PR reviews. It means you only have to review for functionality, formatting is guaranteed through a CI check meaning you don't have to worry about that part in your review. It is basically the python equivalent to running cargo fmt like they do in the main Meilisearch rust project. Still totally your call, I just wanted to let you know what you would be getting in return.

[alallema]

Hi @Dark-Rock,
Thank you so much for this PR!

Thank @sanders41 for all information you provide 🚀

Black should be added as a dev dependency and also a CI check, otherwise, things will get out of sync again.

I think so too, I see that a PR has been created for this purpose 🎉 thanks again @Dark-Rock for this and it seems to be following all the advice you give I will review it soon.

The default line length for black is 88 so the 100 is just my personal preference

I don't really have an opinion on question 88 seems to me like 100, maybe the shorter one would be easier to read.

I would also add isort

Oh, I like to have import sort 😍

mypy and pylint setting could also be moved to the pyproject.toml, but that is an easy follow-up PR.

Yes, perfect I would add an issue about it at the end.

[alallema]

Thanks @Dark-Rock for this PR!
Everything looks good to me but as @sanders41 suggested it:

Black should be added as a dev dependency and also a CI check,

It should be really nice to add black as a dev dependency and also add an example of a command line in the CONTRIBUTING to explain how to run it.
You can take examples on other contributing guides if you need for example dotnet or go

[alallema]

Hi @Dark-Rock,
Thanks for your changes, I'm sorry I should tell you that you have to add it in setup.py but also to Pipfile and commit the changes of Pipfile.lock too.
You should be able to add it with the command:

pipenv install --dev <package-name>

Good job for the README ✨

[alallema]

Hi @Dark-Rock, Pipfile.lock is still missing, also I'm sorry I will merge a new PR with modifications.

[alallema]

@Dark-Rock perfect the dependencies are in a good place! I just add few comments 😃

[sanders41]

lgtm

[alallema]

Thank you very much @Dark-Rock for this particularly tedious PR ❤️
LGTM! 🔥 🔥 🔥

[alallema]

bors merge

[bors]

Build succeeded:

• integration-tests (3.10)
• integration-tests (3.7)
• integration-tests (3.8)
• integration-tests (3.9)
• mypy
• pylint

[alallema]

This message is sent automatically

Thanks again for contributing to Meilisearch ❤️
If you are participating in Hacktoberfest, and you would like to receive some gift from Meilisearch too, please complete this form.