-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added black and isort #1734
Added black and isort #1734
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1734 +/- ##
==========================================
- Coverage 90.90% 90.89% -0.02%
==========================================
Files 59 59
Lines 14229 14229
==========================================
- Hits 12935 12933 -2
- Misses 1294 1296 +2
Continue to review full report at Codecov.
|
@chayim I would propose to merge this asap, because it changes the format of many files, hence will need rerunning |
@WisdomPill First off amazing. thank you! There will be some delay as there's a hefty PR I'd like to merge in first (Thursday ideally). Apologies on that. Secondly - I'd like to remove the pre-commit functionality. It doesn't really work with the current workflow, in that dependencies are declared there that can be different from the dev_requirements.txt. I don't love having a requirements.txt, dev_requirements.txt, and setup.py - but adding a fourth place is IMHO asking too much of future contributors. What do you think? |
I am also not a big fan of pre-commit, as nice as it seems to be, I find it cumbersome, it needs to be synced with ci and the rest of the toolchain (tox and so on). For me it is not a big deal to have the CI complain if the linters are not happy. I would gladly remove it |
I think Line 25 in 4db85ef
Not sure if it will make a difference in output here, though :) |
4b26288
to
86ce5e4
Compare
@akx thanks for pointing that out, I fixed it and rebased based on the latest changes in master |
dd8c428
to
3a9daf4
Compare
@chayim here you go! 😄 |
@WisdomPill Awesome possum. Thank you so much my friend, you're a scholar, and gentlebeing. Incoming to master. |
Fixes: #1644
Pull Request check-list
$ tox
pass with this change (including linting)?Description of change
For the moment I just added
isort
andblack
, there is a problem with black because it produces different code.For the huge changes, I used
black
andisort
that could be used to change the code to make it comply.Note that python3.9 is not yet supported as target in black.