astro-informatics / s2fft

Differentiable and accelerated spherical transforms with JAX
https://astro-informatics.github.io/s2fft
MIT License
134 stars 9 forks source link

Decide on a code style convention and set-up automatic formatting #205

Closed matt-graham closed 1 month ago

matt-graham commented 5 months ago

We don't currently specify the code style convention we want the package code to adhere to. Ideally we should settle on something and document this to make it easy for contributors to know what to target when proposing changes and to ensure we are consistent across the repository to improve readability. This has come up as part of @ASKabalan's contributions in #204.

Currently I think we are mainly following PEP8 guidelines and this is probably the most commonly used convention in open-source Python projects so I would personally suggest adopting this. Alternatives include the Google Python style guide.

Related to this, to simplify keeping to a standard style, it would be good to adopt a specific formatter which enforces our chosen style conventions - as an additional check as part of the CI workflows and/or by setting up pre-commit hooks to automatically run the formatter on any commits. My suggestion for a formatter would be the Ruff formatter as its very performant so minimizes overhead when formatting and uses the same formatting rules as Black which is widely used.

Tagging @jasonmcewen and @CosmoMatt for their views.

ASKabalan commented 5 months ago

I am ok with any formatter. the Ruff formatter seems ok for me

ASKabalan commented 5 months ago

I also add the fact that I chose the c++ formatter to be google with clang-format, using LSST guidelines formatting rules

matt-graham commented 1 month ago

@jasonmcewen, @CosmoMatt and myself discussed this in a meeting today and there was a consesus for using the Black code style.

jasonmcewen commented 1 month ago

Thanks @matt-graham. Yes, since we're using Black in other codes and its fairly common for Python we thought we should go with that. We have been using Black already with s2fft I belive, although perhaps not extensively. We should indeed try to get this fully set up and specified in any contributor guidelines.