zincware / MDSuite

A post-processing engine for particle simulations
https://mdsuite.readthedocs.io/
Eclipse Public License 2.0
36 stars 7 forks source link

replace flake8 with ruff #567

Closed PythonFZ closed 1 year ago

PythonFZ commented 1 year ago

Flake8 had some recent issues with silently removing their gitlab repo. Furthermore, it doesn't look like flake8 will provide support for pyproject.toml any time soon.

https://github.com/charliermarsh/ruff is an alternativ with is noticeably faster and rapidly gaining in popularity. I suggest replacing flake8 with ruff here. I already did this for other packages without issues.

I enabled more warnings, so this PR will most likely fail and we should agree on which errors we want to ignore and which we want to fix.

PythonFZ commented 1 year ago

I disabled most of the failing ones in 9bfdf34 and 00734bb

Personally, I would prefere to keep the excluded list as small as possible and use noqa D123 in the code if there is a reason to not follow the conventions. But I agree that for this PR we probably should merge ruff first and then takle each warning seperatly.

SamTov commented 1 year ago

I don't mind the idea of switching, I would throw one point up though. In terms of longevity, flake8 is run by an organization. Sure it is loosely an organization but it will likely be kept alive. The ruff linter currently is affiliated only with this one developer and so for all e.g Python upgrades we will need to rely on somebody making that work. If the library is quickly being adopted by the community than this is likely not an issue as enough people will need it. But it is a risk we should agree on to some extent. As flake8 has caused no issue in MDSuite, is it worth moving to something that could prove to be less stable? @christophlohrmann @PythonFZ

PythonFZ commented 1 year ago

I don't mind the idea of switching, I would throw one point up though. In terms of longevity, flake8 is run by an organization. Sure it is loosely an organization but it will likely be kept alive. The ruff linter currently is affiliated only with this one developer and so for all e.g Python upgrades we will need to rely on somebody making that work. If the library is quickly being adopted by the community than this is likely not an issue as enough people will need it. But it is a risk we should agree on to some extent. As flake8 has caused no issue in MDSuite, is it worth moving to something that could prove to be less stable? @christophlohrmann @PythonFZ

The ruff package currently has 46 contributers so it is not only maintained by a single person. Flake8 and ruff implement the same lintings / ruff is a replication of flake8 written in rust. Ruff (3.5k stars) already has more stars on github than flake8 (2.5k stars).

I don't see any reason to stick with flake8 tbh. I fully believe that the popularity of ruff would, even if charliermarsh decides to delete the repo lead to forks that keep the package alive.

SamTov commented 1 year ago

I don't mind the idea of switching, I would throw one point up though. In terms of longevity, flake8 is run by an organization. Sure it is loosely an organization but it will likely be kept alive. The ruff linter currently is affiliated only with this one developer and so for all e.g Python upgrades we will need to rely on somebody making that work. If the library is quickly being adopted by the community than this is likely not an issue as enough people will need it. But it is a risk we should agree on to some extent. As flake8 has caused no issue in MDSuite, is it worth moving to something that could prove to be less stable? @christophlohrmann @PythonFZ

The ruff package currently has 46 contributers so it is not only maintained by a single person. Flake8 and ruff implement the same lintings / ruff is a replication of flake8 written in rust. Ruff (3.5k stars) already has more stars on github than flake8 (2.5k stars).

I don't see any reason to stick with flake8 tbh. I fully believe that the popularity of ruff would, even if charliermarsh decides to delete the repo lead to forks that keep the package alive.

Sure but I did not say maintained by a single person I said affiliated with a single person which it is. The adoption by the community is promising, it's not like I am saying I don't want it. But we should perhaps put it to a vote. At the end of the day, if we find an issue we can move to a different linter again.

PythonFZ commented 1 year ago

Then let's have a vote @SamTov @christophlohrmann @Fratorhe

christophlohrmann commented 1 year ago

Since @PythonFZ seems to be much in favour and there is little to no effort involved in going back I vote for the switch even though I don't find it strictly necessary as long as there is not an actual problem concerning mdsuite

SamTov commented 1 year ago

Since @PythonFZ seems to be much in favour and there is little to no effort involved in going back I vote for the switch even though I don't find it strictly necessary as long as there is not an actual problem concerning mdsuite

Fully agree, Fabian if you are a positive vote than we can go ahead