ReactionMechanismGenerator / AutoTST

AutoTST: A framework to perform automated transition state theory calculations
Other
32 stars 16 forks source link

Converted "".format() to f-string literals #67

Closed rwest closed 4 years ago

rwest commented 4 years ago

These are nicer to look at, easier to debug, etc.

I did these using the "convert to f-string literal" code intention in PyCharm. Click inside a .format() string somewhere and press Alt-Enter on your keyboard, then select "convert to f-string literal".

This first commit I just did a couple of files.

There are probably some tools to do it faster than one at a time, like https://github.com/asottile/pyupgrade

Think about when best to do this in regards other open pull requests, to minimize merge conflicts. But I think it's worth doing - either with one concerted effort or gradually as you touch code.

codecov[bot] commented 4 years ago

Codecov Report

Merging #67 into master will increase coverage by <.01%. The diff coverage is 45.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
+ Coverage   63.77%   63.78%   +<.01%     
==========================================
  Files          27       27              
  Lines        4663     4667       +4     
==========================================
+ Hits         2974     2977       +3     
- Misses       1689     1690       +1
Impacted Files Coverage Δ
autotst/calculator/vibrational_analysis_test.py 83.07% <ø> (ø) :arrow_up:
autotst/calculator/vibrational_analysis.py 86.3% <0%> (ø) :arrow_up:
autotst/data/update.py 10.31% <0%> (ø) :arrow_up:
autotst/geometry.py 88.63% <0%> (ø) :arrow_up:
autotst/species.py 71.25% <100%> (ø) :arrow_up:
autotst/job/job_test.py 98.96% <100%> (ø) :arrow_up:
autotst/data/base.py 38.4% <22%> (ø) :arrow_up:
autotst/conformer/systematic.py 61.2% <33.33%> (ø) :arrow_up:
autotst/job/job.py 44.1% <48.78%> (ø) :arrow_up:
autotst/data/inputoutput.py 65.25% <50%> (ø) :arrow_up:
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bb67c2d...16ca1ec. Read the comment docs.

nateharms commented 4 years ago

Definitely a good move with this PR. But after browsing through the Travis CI for the branch, it looks like .format was substituted with . rather than nothing. Thanks for the effort though!

rwest commented 4 years ago

Fixed the mistake (I had done one by hand because it was split across lines, and made a manual error). So I think this shows the PyCharm method is quite effective. Although it reduces it to a few clicks/keystrokes per instance, you still have to do each instance separately, so it might be worth looking into a wholesale approach.

Either way, I'll leave this here for you to decide when best to do it, in relation to your other changes.

nateharms commented 4 years ago

I'm liking f-strings as opposed to .format so I think we should work on this. I've also looking into a package called flynt and it's doing a good job on this. I'm working on some of this right now and I'l push some changes in a bit.

nateharms commented 4 years ago

Okay, just pushed some changes, let me know thoughts

nateharms commented 4 years ago

Fixed a couple of typos and did a rebase to truncate those into their proper PRs. Let's see how the tests go but I think it should be ready to merge in shortly

nateharms commented 4 years ago

Okay, tests pass and everything's looking good. Unless @davidfarinajr has any complaints, I'll merge this in shortly

nateharms commented 4 years ago

Well, there's also the codecov/patch not passing... but that doesn't really bother me too much.

davidfarinajr commented 4 years ago

Sorry, not sure how I missed this earlier today. Looks good, you can merge.