Skip to content

Commit

Permalink
Replace old format string interpolation with f-strings (#579)
Browse files Browse the repository at this point in the history
Since Python 3.5 is no longer supported, format string interpolations
can now be replaced by f-strings, introduced in Python 3.6, which are
more readable, require less characters and are more efficient.

Note that `pylint` issues a warning when using f-strings for log
messages, just as it does for format interpolated strings. The reasoning
is that this is slightly inefficient as the strings are always
interpolated even if the log is discarded, but also by not passing the
formatting parameters as arguments, the available metadata is reduced.
I feel these inefficiencies are premature optimizations as they are
really minimal and don't weigh up against the improved readability and
maintainability of using f-strings. That is why the `pylint` config is
update to ignore the warning `logging-fstring-interpolation` which
replaces `logging-format-interpolation` that was ignored before.

The majority of the conversions were done automatically with the linting
tool `flynt` which is also added as a pre-commit hook. It is added
before the `yapf` step because since `flynt` will touch formatting,
`yapf` will then get a chance to check it.
  • Loading branch information
sphuber committed Nov 17, 2020
1 parent 947b1fc commit 74d4568
Show file tree
Hide file tree
Showing 53 changed files with 271 additions and 306 deletions.
9 changes: 9 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ repos:
tests/.*.in$
)$
- repo: https://github.com/ikamensh/flynt/
rev: '0.55'
hooks:
- id: flynt
args: [
'--line-length=120',
'--fail-on-change',
]

- repo: https://github.com/PyCQA/pydocstyle
rev: 5.0.2
hooks:
Expand Down
30 changes: 13 additions & 17 deletions aiida_quantumespresso/calculations/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from aiida import orm
from aiida.common import datastructures, exceptions
from aiida.common.lang import classproperty

from qe_tools.converters import get_parameters_from_cell

from aiida_quantumespresso.utils.convert import convert_input_to_namelist_entry
Expand Down Expand Up @@ -163,7 +162,7 @@ def prepare_for_submission(self, folder):

# Create an `.EXIT` file if `only_initialization` flag in `settings` is set to `True`
if settings.pop('ONLY_INITIALIZATION', False):
with folder.open('{}.EXIT'.format(self._PREFIX), 'w') as handle:
with folder.open(f'{self._PREFIX}.EXIT', 'w') as handle:
handle.write('\n')

# Check if specific inputs for the ENVIRON module where specified
Expand Down Expand Up @@ -240,7 +239,7 @@ def prepare_for_submission(self, folder):

if settings:
unknown_keys = ', '.join(list(settings.keys()))
raise exceptions.InputValidationError('`settings` contained unexpected keys: {}'.format(unknown_keys))
raise exceptions.InputValidationError(f'`settings` contained unexpected keys: {unknown_keys}')

return calcinfo

Expand Down Expand Up @@ -332,7 +331,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
)

kind_names.append(kind.name)
atomic_species_card_list.append('{} {} {}\n'.format(kind.name.ljust(6), kind.mass, filename))
atomic_species_card_list.append(f'{kind.name.ljust(6)} {kind.mass} {filename}\n')

# I join the lines, but I resort them using the alphabetical order of
# species, given by the kind_names list. I also store the mapping_species
Expand Down Expand Up @@ -370,13 +369,11 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
for i, this_atom_fix in enumerate(fixed_coords):
if len(this_atom_fix) != 3:
raise exceptions.InputValidationError(
'fixed_coords({:d}) has not length three'
''.format(i + 1))
f'fixed_coords({i + 1:d}) has not length three')
for fixed_c in this_atom_fix:
if not isinstance(fixed_c, bool):
raise exceptions.InputValidationError(
'fixed_coords({:d}) has non-boolean '
'elements'.format(i + 1))
f'fixed_coords({i + 1:d}) has non-boolean elements')

if_pos_values = [cls._if_pos(_) for _ in this_atom_fix]
fixed_coords_strings.append(
Expand Down Expand Up @@ -418,7 +415,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin

# Checking that all 3 dimensions are specified:
if len(vector) != 3:
raise exceptions.InputValidationError('Forces({}) for {} has not length three'.format(vector, site))
raise exceptions.InputValidationError(f'Forces({vector}) for {site} has not length three')

lines.append('{0} {1:18.10f} {2:18.10f} {3:18.10f}\n'.format(site.kind_name.ljust(6), *vector))

Expand All @@ -444,7 +441,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
# Checking that all 3 dimensions are specified:
if len(vector) != 3:
raise exceptions.InputValidationError(
'Velocities({}) for {} has not length three'.format(vector, site)
f'Velocities({vector}) for {site} has not length three'
)

lines.append('{0} {1:18.10f} {2:18.10f} {3:18.10f}\n'.format(site.kind_name.ljust(6), *vector))
Expand All @@ -467,7 +464,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
tolerance=settings.pop('IBRAV_CELL_TOLERANCE', 1e-6)
)
except ValueError as exc:
raise QEInputValidationError('Cannot get structure parameters from cell: {}'.format(exc)) from exc
raise QEInputValidationError(f'Cannot get structure parameters from cell: {exc}') from exc
input_params['SYSTEM'].update(structure_parameters)
input_params['SYSTEM']['nat'] = len(structure.sites)
input_params['SYSTEM']['ntyp'] = len(structure.kinds)
Expand Down Expand Up @@ -526,7 +523,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
else:
kpoints_type = 'crystal'

kpoints_card_list = ['K_POINTS {}\n'.format(kpoints_type)]
kpoints_card_list = [f'K_POINTS {kpoints_type}\n']

if kpoints_type == 'automatic':
if any([i not in [0, 0.5] for i in offset]):
Expand All @@ -540,11 +537,10 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin
# nothing to be written in this case
pass
else:
kpoints_card_list.append('{:d}\n'.format(num_kpoints))
kpoints_card_list.append(f'{num_kpoints:d}\n')
for kpoint, weight in zip(kpoints_list, weights):
kpoints_card_list.append(
' {:18.10f} {:18.10f} {:18.10f} {:18.10f}'
'\n'.format(kpoint[0], kpoint[1], kpoint[2], weight))
f' {kpoint[0]:18.10f} {kpoint[1]:18.10f} {kpoint[2]:18.10f} {weight:18.10f}\n')

kpoints_card = ''.join(kpoints_card_list)
del kpoints_card_list
Expand Down Expand Up @@ -577,7 +573,7 @@ def _generate_PWCPinputdata(cls, parameters, settings, pseudos, structure, kpoin

inputfile = ''
for namelist_name in namelists_toprint:
inputfile += '&{0}\n'.format(namelist_name)
inputfile += f'&{namelist_name}\n'
# namelist content; set to {} if not present, so that we leave an empty namelist
namelist = input_params.pop(namelist_name, {})
for key, value in sorted(namelist.items()):
Expand Down Expand Up @@ -623,7 +619,7 @@ def _case_transform_dict(dictionary, dict_name, func_name, transform):
from collections import Counter

if not isinstance(dictionary, dict):
raise TypeError('{} accepts only dictionaries as argument, got {}'.format(func_name, type(dictionary)))
raise TypeError(f'{func_name} accepts only dictionaries as argument, got {type(dictionary)}')
new_dict = dict((transform(str(k)), v) for k, v in dictionary.items())
if len(new_dict) != len(dictionary):
num_items = Counter(transform(str(k)) for k in dictionary.keys())
Expand Down
10 changes: 5 additions & 5 deletions aiida_quantumespresso/calculations/cp.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class CpCalculation(BasePwCpInputGenerator):
_FILE_XML_PRINT_COUNTER_BASENAME = 'print_counter.xml'
_FILE_XML_PRINT_COUNTER = os.path.join(
BasePwCpInputGenerator._OUTPUT_SUBFOLDER,
'{}_{}.save'.format(BasePwCpInputGenerator._PREFIX, _CP_WRITE_UNIT_NUMBER),
f'{BasePwCpInputGenerator._PREFIX}_{_CP_WRITE_UNIT_NUMBER}.save',
_FILE_XML_PRINT_COUNTER_BASENAME,
)

Expand Down Expand Up @@ -90,20 +90,20 @@ class CpCalculation(BasePwCpInputGenerator):
_internal_retrieve_list = [
os.path.join(
BasePwCpInputGenerator._OUTPUT_SUBFOLDER,
'{}.{}'.format(BasePwCpInputGenerator._PREFIX, ext),
f'{BasePwCpInputGenerator._PREFIX}.{ext}',
) for ext in _cp_ext_list
] + [_FILE_XML_PRINT_COUNTER]

# in restarts, it will copy from the parent the following
_restart_copy_from = os.path.join(
BasePwCpInputGenerator._OUTPUT_SUBFOLDER,
'{}_{}.save'.format(BasePwCpInputGenerator._PREFIX, _CP_WRITE_UNIT_NUMBER),
f'{BasePwCpInputGenerator._PREFIX}_{_CP_WRITE_UNIT_NUMBER}.save',
)

# in restarts, it will copy the previous folder in the following one
_restart_copy_to = os.path.join(
BasePwCpInputGenerator._OUTPUT_SUBFOLDER,
'{}_{}.save'.format(BasePwCpInputGenerator._PREFIX, _CP_READ_UNIT_NUMBER),
f'{BasePwCpInputGenerator._PREFIX}_{_CP_READ_UNIT_NUMBER}.save',
)

@classproperty
Expand All @@ -115,7 +115,7 @@ def xml_filepaths(cls):
for filename in cls.xml_filenames:
filepath = os.path.join(
cls._OUTPUT_SUBFOLDER,
'{}_{}.save'.format(cls._PREFIX, cls._CP_WRITE_UNIT_NUMBER),
f'{cls._PREFIX}_{cls._CP_WRITE_UNIT_NUMBER}.save',
filename,
)
filepaths.append(filepath)
Expand Down
8 changes: 4 additions & 4 deletions aiida_quantumespresso/calculations/epw.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_offset(offset):
parent_calc_nscf = parent_folder_nscf.creator

if parent_calc_nscf is None:
raise exceptions.NotExistent('parent_folder<{}> has no parent calculation'.format(parent_folder_nscf.pk))
raise exceptions.NotExistent(f'parent_folder<{parent_folder_nscf.pk}> has no parent calculation')

# Also, the parent calculation must be on the same computer
if not self.node.computer.uuid == parent_calc_nscf.computer.uuid:
Expand Down Expand Up @@ -188,13 +188,13 @@ def test_offset(offset):

# add here the list of point coordinates
if len(list_of_points) > 1:
postpend_text = '{} cartesian\n'.format(len(list_of_points))
postpend_text = f'{len(list_of_points)} cartesian\n'
for points in list_of_points:
postpend_text += '{0:18.10f} {1:18.10f} {2:18.10f} \n'.format(*points)

with folder.open(self.metadata.options.input_filename, 'w') as infile:
for namelist_name in namelists_toprint:
infile.write('&{0}\n'.format(namelist_name))
infile.write(f'&{namelist_name}\n')
# namelist content; set to {} if not present, so that we leave an empty namelist
namelist = parameters.pop(namelist_name, {})
for key, value in sorted(namelist.items()):
Expand Down Expand Up @@ -282,6 +282,6 @@ def test_offset(offset):

if settings:
unknown_keys = ', '.join(list(settings.keys()))
raise exceptions.InputValidationError('`settings` contained unexpected keys: {}'.format(unknown_keys))
raise exceptions.InputValidationError(f'`settings` contained unexpected keys: {unknown_keys}')

return calcinfo
Loading

0 comments on commit 74d4568

Please sign in to comment.