search

bbfrederick / rapidtide

rapidtide - a suite of programs for doing time lag correlation analysis on fMRI data
Apache License 2.0
75 stars 14 forks source link

Run black automated code formatter on package #48

Closed tsalo closed 3 years ago

tsalo commented 4 years ago

This stems from https://github.com/bbfrederick/rapidtide/pull/46#issuecomment-717448734. Prior to releasing 2.0.0, we should run black on the full package to standard the formatting of the code. This will have the added benefit of minimizing future diffs to better isolate relevant changes.

tsalo commented 3 years ago

@bbfrederick are you planning to deal with the merge conflicts between dev and master when you release 2.0.0, or will you just have dev overwrite master? I ask because running black on dev would introduce a massive number of merge conflicts if you're planning to merge the two branches manually.

bbfrederick commented 3 years ago

I was planning to simply overwrite master with dev. Is there a reason that would be a bad thing?

tsalo commented 3 years ago

It could be a problem if you push changes to master that you ultimately wanted supported in dev, but then didn't reimplement on dev. Other than that, I don't think there should be any issues.

tsalo commented 3 years ago

In that case, it seems like it should be okay to start running black at any time. Given the sheer size of rapidtide, it might be good to do it in a series of PRs so that the changes are more digestible (unless you want to run it all at once yourself). Would it be cool for me to start submitting style PRs? If so, are there any modules I should avoid touching at the moment?

bbfrederick commented 3 years ago

I've only backported critical fixes I've made to dev to master for several months, so I don't think that will be a problem.

Re: starting to do style PR's, go for it. The files I'm working on most actively at the moment are workflows/rapidtide.py, workflows/rapidtide_parser.py, workflows/rapidtide2x_parser.py, workflows/happy.py, refine.py and stats.py. Anything else you can probably start doing in chunks and I'd never notice. For example, the entire scripts directory.

tsalo commented 3 years ago

I ran into some technical issues a few days ago but I'll hopefully be able to start opening PRs by the end of the week.

tsalo commented 3 years ago

After a bit more thought, I'm not sure if I should run black. It would artificially make it seem like I rewrote the whole library in the commit history. Would you be willing to run black and push the changes yourself instead? You can pip install black and then to run it you just do black /path/to/rapidtide/ from the command line.

tsalo commented 3 years ago

Closed by 042d2cf712245a31d162e5950c21670b92f2f1da.

bbfrederick commented 3 years ago

Just so you know, I've enabled the Black integration into PyCharm, so this should just happen automatically going forward.