allenai / allennlp

An open-source NLP research library, built on PyTorch.
http://www.allennlp.org
Apache License 2.0
11.76k stars 2.25k forks source link

Use isort in CI #4989

Open epwalsh opened 3 years ago

epwalsh commented 3 years ago

https://pypi.org/project/isort/

Sorts imports according to PEP 8. Compatible with black.

epwalsh commented 3 years ago

I looked into this. All we really care about is adhering to PEP 8, which for imports just means they are grouped by module type (std lib, third party, and local). We don't care about the ordering within groups.

isort has a flag --only-sections which sounds like what we want, i.e. to only enforce that imports are grouped together by module type. But when using this flag, isort still sorts by import style (from x import y vs import x) within groups/sections. This may be a bug, so I've filed an issue (https://github.com/PyCQA/isort/issues/1675).

If that is in fact a bug and it gets resolved, then I think we should move forward with this. Otherwise it's more trouble than it's worth and would annoy @dirkgr a lot.

dirkgr commented 3 years ago

So we sit tight until we hear back about the issue? In that case I'll assign this to milestone 2.2 so we don't forget about it.

epwalsh commented 3 years ago

Yeup, 2.2 sounds good.