google-deepmind / meltingpot

A suite of test scenarios for multi-agent reinforcement learning.
Apache License 2.0
614 stars 122 forks source link

Pylint #50

Closed alan-cooney closed 2 years ago

alan-cooney commented 2 years ago

Hi - what's the standard practice for pylintrc files with Deep Mind projects?

Could we add the standard Google one to the repo, and perhaps also add a linting step the CD? Happy to commit a PR if useful.

jagapiou commented 2 years ago

Hi Alan, I'll add a .pylintrc and push that out. Our internal tooling means that i don't have a great idea what's best for the external workflow; so if you could point me at something (or do a PR once the .pylintrc is in) that would be great.

Thank you :-)

alan-cooney commented 2 years ago

Thanks John!

Something like https://github.com/marketplace/actions/python-code-quality-and-lint should work

willis-richard commented 2 years ago

Should the .style.yapf not use equals instead of colons? i.e.

[style]
based_on_style = chromium
indent_width = 2 
split_before_named_assigns = True
column_limit = 80
willis-richard commented 2 years ago

Actually, I am still getting errors using this file. Would you be able to advise me please? I believe this is because chromium is not a recognised style.

Yapf says that you can choose a predefined style https://github.com/google/yapf ... based in the file style.py https://github.com/google/yapf/blob/main/yapf/yapflib/style.py But chromium is not one of them.

willis-richard commented 2 years ago

Update: I was wrong about the equals signs, colons are fine. But I think you want based_on_style: yapf That appears to be what the chromium style is now called https://github.com/google/yapf/pull/789

jagapiou commented 2 years ago

Thanks for the heads-up, I'd just copy-pasted it from elsewhere. I think it should just be:

[style]
based_on_style: yapf

Change incoming soon.