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

More f-string conversion using pyupgrade 2.25.0 #3724

Merged
merged 4 commits into from
Sep 11, 2021

Conversation

peterjc
Copy link
Member

@peterjc peterjc commented Sep 10, 2021

  • I hereby agree to dual licence this and any previous contributions under both
    the Biopython License Agreement AND the BSD 3-Clause License.

  • I have read the CONTRIBUTING.rst file, have run pre-commit locally,
    and understand that AppVeyor and TravisCI will be used to confirm the Biopython unit
    tests and style checks pass with these changes.

  • I have added my name to the alphabetical contributors listings in the files
    NEWS.rst and CONTRIB.rst as part of this pull request, am listed
    already, or do not wish to be listed. (This acknowledgement is optional.)

Progress on #2510, follows on from #3723. With these changes about a quarter of the main code base string formatting will be f-strings:

$ flake8 --isolated --select SFS --statistics BioSQL Bio
...
1297  SFS101 String literal formatting using percent operator.
28    SFS201 String literal formatting using format method.
406   SFS301 String literal formatting using f-string.

(pyupgrade would convert about 100 more of the percent uses, but mostly to format strings - there about 30 more f-strings it would create which I missed in this manual selection)

Ran this:

$ pyupgrade --py37-plus Bio/*.py Bio/*/*.py Bio/*/*/*.py BioSQL/*.py
$ black Bio/ BioSQL/

Then reverted any changes adding a .format method, spotted with:

$ git diff | grep "^+" | grep -E "(\.format|\+\+\+)"
Used pyupgrade then reverted most of the more complicated
examples where it wanted to use a .format call, but turned
a few others into f-strings by hand.
@peterjc
Copy link
Member Author

peterjc commented Sep 10, 2021

I think on balance I prefer the behaviour of flynt in creating f-strings, but some of the other changes from pyupgrade are worthwhile - see #3722.

Expect a followup PR using flynt to follow (I want to keep the size of change sets reasonable for checking by eye).

@peterjc
Copy link
Member Author

peterjc commented Sep 10, 2021

Exciting, a failure:

======================================================================
ERROR: test_str (test_Blast_Record.TestHsp)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/circleci/project/biopython-1.80.dev0/Tests/test_Blast_Record.py", line 28, in test_str
    "\n".join(l.strip() for l in str(hsp).split("\n")),
  File "/home/circleci/project/biopython-1.80.dev0/build/lib.linux-x86_64-3.9/Bio/Blast/Record.py", line 245, in __str__
    lines.append(f"Query:{self.query_start:>8} {self.query} {self.query_end}")
TypeError: unsupported format string passed to NoneType.__format__

----------------------------------------------------------------------

Was:

lines.append(
                "Query:%8s %s %s" % (self.query_start, self.query, self.query_end)
)

PR used:

lines.append(f"Query:{self.query_start:>8} {self.query} {self.query_end}")

If the value is None, then %s gives the string "None", likewise %8s with padding:

>>> "%8s" % None
'    None'

However, while f"{None}" gives the string too, the string align formatting fails:

>>> f"{None:>8}"
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: unsupported format string passed to NoneType.__format__

@codecov
Copy link

codecov bot commented Sep 10, 2021

Codecov Report

Merging #3724 (6b52163) into master (70aaa34) will decrease coverage by 1.69%.
The diff coverage is 43.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3724      +/-   ##
==========================================
- Coverage   83.77%   82.08%   -1.70%     
==========================================
  Files         314      294      -20     
  Lines       52512    49392    -3120     
==========================================
- Hits        43994    40543    -3451     
- Misses       8518     8849     +331     
Impacted Files Coverage Δ
Bio/AlignIO/FastaIO.py 93.93% <0.00%> (ø)
Bio/AlignIO/MsfIO.py 66.17% <0.00%> (ø)
Bio/Blast/Applications.py 84.39% <0.00%> (ø)
Bio/Blast/NCBIXML.py 91.96% <0.00%> (ø)
Bio/ExPASy/Prosite.py 81.50% <0.00%> (ø)
Bio/ExPASy/ScanProsite.py 30.15% <0.00%> (ø)
Bio/ExPASy/__init__.py 34.61% <0.00%> (ø)
Bio/File.py 87.20% <0.00%> (ø)
Bio/HMM/MarkovModel.py 89.47% <ø> (ø)
Bio/HMM/Trainer.py 96.61% <0.00%> (ø)
... and 72 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 70aaa34...6b52163. Read the comment docs.

The input value can be None.
@peterjc
Copy link
Member Author

peterjc commented Sep 11, 2021

Cross reference https://bugs.python.org/issue45165 for the use of f-string alignment with None.

@peterjc peterjc merged commit 851003c into biopython:master Sep 11, 2021
@peterjc peterjc deleted the pyupgrade_more branch September 11, 2021 11:30
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.

None yet

2 participants