activitysim
activitysim copied to clipboard
Black and iSort Code Formatting
As we have more contributors to the code, we should begin to enforce code formatting. The review of the SANDAG cross-border model and other enhancements revealed code changes that added line breaks and other formatting changes. Code format is inherently biased by the person writing the code, and we could end up with lots of formatting changes in the Github repo rather than substantive changes over the long term.
This is a low priority request, but it is also super easy to implement.
We do have some basic code formatting enforced currently by pycodestyle, it's not like it's the wild west out here currently. I'm not fundamentally opposed to black instead, but suggesting it's "easy" is leaving out a lot of work for anyone who's working on code updates that don't get merged first. This is a huge project already with at least 57 known forks, and doing this is like dropping a glitter bomb on anyone working on modified versions of the code -- it shouldn't actually break anything but it'll make a huge mess to clean up.
I like this as it makes sure that future diffs are really...diffs. I suggest the following:
- add black as a pre-commit hook in main and develop
- add a commit that is purely a "cleanup" commit with a message reminding PRs to run black
- when developers update their forks to pull back to develop (which they should do), they will also add the pre-commit hook which will run on any updates to their PR.
Just my 2cents
I like @e-lo suggestion of a pre-commit hook. I'm not necessarily wed to Black, but I do feel like we need a format enforcer like Black or AutoPep8. It eliminates any subjected, personal formatting bias (in addition to ensuring diffs are diffs). Removing that subjective bias is great, because I don't ever have to call-out someone for using a tab instead of 4 spaces and other nitpicky formatting issues. I'm sure my syntax is always awesome, and it never annoys anyone though. :)
This is a good article on how to set it all up using the pre-commit hooks.
We agreed on the 10/7 call to try this after all the open PRs are merged and we're about to release the next version since this will be the least inconvenient moment.
I've discovered an issue with pre-commit that makes it unappealing for use on Windows when also using conda. https://github.com/pre-commit/pre-commit/issues/1329
I've discovered an issue with pre-commit that makes it unappealing for use on Windows when also using conda. https://github.com/pre-commit/pre-commit/issues/1329
Should be fixed now 🥳