Auto-formatting with black and isort

[araffin]

Description

Motivation and Context

• I have raised an issue to propose this change (required for new features and bug fixes)

Related to #17

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (update in the documentation)

Checklist:

• I've read the CONTRIBUTION guide (required)
• I have updated the changelog accordingly (required).
• My change requires a change to the documentation.
• I have updated the tests accordingly (required for a bug fix or a new feature).
• I have updated the documentation accordingly.
• I have checked the codestyle using make lint
• I have ensured make pytest and make type both pass.

[Miffyli]

Looking at the changes I must say I quite like black's formatting. There are some parts I do not like (much like you discussed in #17), but overall I prefer it much more over not having a fixed standard across the code. I also like how it changes ' to " :).

I give this a green light, but would like someone else to give in a review, given how much this will affect the whole codebase in the future.

[araffin]

Looking at the changes I must say I quite like black's formatting. There are some parts I do not like (much like you discussed in #17), but overall I prefer it much more over not having a fixed standard across the code. I also like how it changes ' to " :).

Yep, same feeling. But it is nice not to worry or waste time about formatting.

I give this a green light, but would like someone else to give in a review, given how much this will affect the whole codebase in the future.

Yep, It would be nice if all maintainers agree for that one.
@AdamGleave already did, but I did not get any feedback from @erniejunior and @hill-a yet.

[m-rph]

A pre-commit hook that auto-formats would be quite useful.

[araffin]

A pre-commit hook that auto-formats would be quite useful.

not sure about that one... will people also have it when they clone the repo?
Personally, I prefer when nothing is done without me knowing, and doing make format is quite easy.

[AdamGleave]

LGTM.

[AdamGleave]

A pre-commit hook that auto-formats would be quite useful.

not sure about that one... will people also have it when they clone the repo?
Personally, I prefer when nothing is done without me knowing, and doing make format is quite easy.

I do like having commit hooks to run all the cheap checks so I don't have to remember to do it manually (or wait 30m to see the CI fails), but am also fine with writing one myself.

We could add an entry in the Makefile that just runs all the other quick entries? Something like:

commit-checks: type lint format spelling

[hill-a]

LGTM.

Honestly I might just add black and isort to my workspace in general. I didn't realise they existed.

The auto formating looks really good.
My only issue I have is with the function definition format:

    def foo(
        arg1,
        arg2
    ):
        # applies foo to args
        return None

But that probably because I'm not used to it (I keep trying to look for the ): for the end of the function signature).
From what I can tell this is done to avoid large diffs.

[araffin]

But that probably because I'm not used to it (I keep trying to look for the ): for the end of the function signature).
From what I can tell this is done to avoid large diffs.

Yep, that's part of the things I'm not completely ok with it, but that's the compromise (there is no with black ^^").
I guess it will look normal over time.

[araffin]

Can someone tell me why Github CI fails?
For Gitlab, I have an idea... It comes from opencv.