Thanks @Zac-HD

@max-sixty - closed by #3142 I think?

I tried your setup.cfg above but still getting inconsistencies with black when I have comments in the imports.

black: ```python import foo

Let me explain why we do this

import bar ```

isort: ```python import foo

Let me explain why we do this

import bar ``` and I strongly agree with black.

I didn't know about isort, nice. However, from a very quick test, it's not compatible with black

There's a specific isort config that's compatible with black; this is what we use internally (there's a similar official version I believe):

[isort] default_section=THIRDPARTY force_grid_wrap=0 include_trailing_comma=True line_length=88 multi_line_output=3 use_parentheses=True

I just ran isort on my own project. Overall, it gave me a bunch of good suggestions, but the amount of times I twitched my nose and manually reverted what it did are too many for my tastes.

Besides the aforementioned incompatibilities with black, I hate when it changes from .. import module1 from .. import module2 to from .. import module1, module2 which in my opinion flies in the face of the PEP8 rule of never importing two modules on the same line.

So my opinion is: +1 for a manually vetted one-time run; -1 for automatic integration in CI.

flake8 is not just about formatting; none of the pyflakes errors (e.g. double/unused imports) are covered by black.

I didn't know about isort, nice. However, from a very quick test, it's not compatible with black e.g. if you run black -> isort -> black, the two tools keep touching each other's changes.

@shoyer great

TODOs (please add where I've missed)

• [ ] Reformat code
• [ ] CI checks
• [ ] Short instructions for how to merge an existing PR (i.e. avoid manual merge resolution)
• [ ] Black badge
• [ ] Do we want to keep flake8 checks? Black is mostly stricter but not always, e.g. on lines at the end of files. (+0.3 from me to use only black and stop using flake8)
• [ ] Do we want to include isort? (-0.1 from me, even though I like the tool)

Let's wait a little while for the burst of activity from SciPy to die down first.

On Mon, Jul 15, 2019 at 8:03 AM Maximilian Roos notifications@github.com wrote:

I'm happy to do this today

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/issues/3092?email_source=notifications&email_token=AAJJFVR2KNJP3PXHPG4ZZK3P7SGTXA5CNFSM4IA3BF72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ57I3A#issuecomment-511439980, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJJFVUQ4OMMUZJ5LQEFF7TP7SGTXANCNFSM4IA3BF7Q .

I'm happy to do this today

Since Black requires Python >=3.6, which would set the minimum Python version to 3.6 for contributing to xarray (which currently requires Python >= 3.5), are there any plans of dropping Python 3.5 support in the near future?

I don't think these decisions need to be coupled. It's OK to make contributors upgrade to a newer version of Python before all our users.

There are no immediate plans to drop Python 3.5 support, but we haven't really discussed it either. I think it would definitely make sense to drop Python 3.5 after Python 3.8 is available, maybe sooner. Four major Python versions would definitely be too many.

Since Black requires Python >=3.6, which would set the minimum Python version to 3.6 for contributing to xarray (which currently requires Python >= 3.5), are there any plans of dropping Python 3.5 support in the near future?

+1 from me, too.

It's definitely going to cause some nasty merge conflicts for any work in progress, but hopefully it's straightforward to fix those by running black on the PRs and then merging in master.

We should be able to simply add another job on Azure for this. The lint task using flake8 on Azure's pre-installed Python runs very quickly (less than 30 seconds!) and I would expect to see the same from black.

If we do this, which I am +1 on, we should add a pre-commit hook so we don't have to think about the manual applications. We should probably just copy the dask approach: https://github.com/dask/dask/pull/4983.

I would be v keen. I find it great. But I'm generally too keen on a) tools and b) changing everything at once, so I discount myself a bit here!

I don't think it's possible to apply incrementally, unfortunately (e.g. only on changed lines). So it would require a single sweeping change. Interested to know how other projects are handling. IIRC they're applying it to some parts of stdlib too.