Skip to content
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

Black formatting #269

Merged
merged 6 commits into from
Aug 25, 2023
Merged

Black formatting #269

merged 6 commits into from
Aug 25, 2023

Conversation

a-ws-m
Copy link
Contributor

@a-ws-m a-ws-m commented Aug 21, 2023

Use black to format two files, per this request.

@VOD555 VOD555 requested a review from orbeckst August 22, 2023 04:21
@codecov
Copy link

codecov bot commented Aug 22, 2023

Codecov Report

Merging #269 (2ddf4cd) into develop (3971e5d) will not change coverage.
The diff coverage is 82.32%.

@@           Coverage Diff            @@
##           develop     #269   +/-   ##
========================================
  Coverage    81.29%   81.29%           
========================================
  Files           15       15           
  Lines         1978     1978           
  Branches       297      297           
========================================
  Hits          1608     1608           
  Misses         278      278           
  Partials        92       92           
Files Changed Coverage Δ
mdpow/restart.py 73.10% <55.55%> (ø)
mdpow/workflows/base.py 83.07% <70.00%> (ø)
mdpow/fep.py 68.77% <74.07%> (ø)
mdpow/config.py 84.82% <75.00%> (ø)
mdpow/run.py 71.58% <78.94%> (ø)
mdpow/equil.py 80.68% <86.48%> (ø)
mdpow/forcefields.py 87.80% <92.30%> (ø)
mdpow/analysis/ensemble.py 97.18% <95.83%> (ø)
mdpow/__init__.py 100.00% <100.00%> (ø)
mdpow/analysis/dihedral.py 100.00% <100.00%> (ø)
... and 5 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@orbeckst orbeckst linked an issue Aug 22, 2023 that may be closed by this pull request
@orbeckst
Copy link
Member

Let's just switch everything over to black #271 — I wanted to try that out in a project anyway.

@a-ws-m could you do the honors and blacken the whole code base, please?

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #271 — please

  • blacken all code (in mdpow, mdpow/tests, and scripts)
  • add yourself to AUTHORS
  • add an entry to CHANGES

@orbeckst orbeckst mentioned this pull request Aug 22, 2023
@a-ws-m
Copy link
Contributor Author

a-ws-m commented Aug 22, 2023

Done, I also:

  • Added a .git-blame-ignore-revs per the suggestion on that link you sent.
  • Added some files in .vscode so that users of that editor will be recommended to install black and have it format Python files upon saving.

@a-ws-m a-ws-m requested a review from orbeckst August 22, 2023 15:23
Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looking good.

There are minor formatting changes for the CHANGES. You can just accept my "suggestions" from the comments.

My original plan was to squash-merge. However, if you're familiar with git then you could rewrite the history of this branch into two commits:

  1. all black changes
  2. all other changes (AUTHORS, CHANGES, workflows, updated .git-blame-ignore-revs with the commit ID of (1))

Then I will be able to just merge without any need for any further changes.

Let me know if you want to rewrite the history. Otherwise I'll squash-merge after the code review suggestions are included and tests pass.

@@ -0,0 +1,4 @@
# Autoformat two files using `black`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding. I'll squah-merge and then rewrite this file afterwards with the proper commit id.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Unless you rewrite the history yourself and add the proper commit ID of the single commit with all black-generated changes. See review comment.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, I'm not confident I'll get that right to be honest with you! If it's OK, I'll leave it to you to change the hashes after merging.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, will do

CHANGES Outdated Show resolved Hide resolved
CHANGES Outdated Show resolved Hide resolved
@orbeckst
Copy link
Member

I rewrote the history.

@orbeckst orbeckst self-assigned this Aug 25, 2023
@orbeckst orbeckst merged commit 6449879 into Becksteinlab:develop Aug 25, 2023
@orbeckst
Copy link
Member

Alright, we're now using black!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use black for unified formatting
2 participants