From 7b55451a309f92d45cc7911d8edcec0977fcd91b Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Mon, 3 Jun 2024 12:12:29 -0700 Subject: [PATCH] add stricter formatting because it was easy --- py/desitransfer/common.py | 11 +++++---- py/desitransfer/daemon.py | 27 +++++++++++----------- py/desitransfer/daily.py | 28 +++++++++++------------ py/desitransfer/nightwatch.py | 2 +- py/desitransfer/test/test_common.py | 2 +- py/desitransfer/test/test_daemon.py | 31 +++++++++++++------------- py/desitransfer/test/test_status.py | 20 ++++++++--------- py/desitransfer/test/test_top_level.py | 3 +-- py/desitransfer/tucson.py | 2 +- setup.cfg | 8 +++---- 10 files changed, 64 insertions(+), 70 deletions(-) diff --git a/py/desitransfer/common.py b/py/desitransfer/common.py index d111b6b..37505a5 100644 --- a/py/desitransfer/common.py +++ b/py/desitransfer/common.py @@ -35,7 +35,7 @@ def empty_rsync(out): ``True`` if there are no files to transfer. """ rr = re.compile(r'(receiving|sent [0-9]+ bytes|total size)') - return all([rr.match(l) is not None for l in out.split('\n') if l]) + return all([rr.match(out_line) is not None for out_line in out.split('\n') if out_line]) def new_exposures(out): @@ -53,8 +53,8 @@ def new_exposures(out): """ e = set() e_re = re.compile(r'([0-9]{8})/?') - for l in out.split('\n'): - m = e_re.match(l) + for out_line in out.split('\n'): + m = e_re.match(out_line) if m is not None: e.add(m.groups()[0]) return e @@ -126,7 +126,7 @@ def ensure_scratch(directories): """ for d in directories: try: - l = os.listdir(d) + dir_list = os.listdir(d) except FileNotFoundError: continue return d @@ -144,7 +144,7 @@ def today(): This formulation, with the offset ``7/24+0.5``, is inherited from previous nightwatch transfer scripts. """ - return (dt.datetime.utcnow() - dt.timedelta(7/24+0.5)).strftime('%Y%m%d') + return (dt.datetime.utcnow() - dt.timedelta(7 / 24 + 0.5)).strftime('%Y%m%d') def idle_time(start=8, end=12, tz=None): @@ -191,4 +191,3 @@ def exclude_years(start_year): A list suitable for appending to a command. """ return (' '.join([f'--exclude {y:d}*' for y in range(start_year, time.localtime().tm_year)])).split() - diff --git a/py/desitransfer/daemon.py b/py/desitransfer/daemon.py index ca67715..202587a 100644 --- a/py/desitransfer/daemon.py +++ b/py/desitransfer/daemon.py @@ -78,11 +78,10 @@ def __init__(self, options): self._ini = options.configuration self.test = options.test self.tape = options.backup - getlist = lambda x: x.split(',') - getdict = lambda x: dict([tuple(i.split(':')) for i in x.split(',')]) self.conf = ConfigParser(defaults=os.environ, strict=True, interpolation=ExtendedInterpolation(), - converters={'list': getlist, 'dict': getdict}) + converters={'list': lambda x: x.split(','), + 'dict': lambda x: dict([tuple(i.split(':')) for i in x.split(',')])}) files = self.conf.read(self._ini) # assert files[0] == self._ini self.sections = [s for s in self.conf.sections() @@ -183,11 +182,11 @@ def directory(self, d): _, out, err = _popen(cmd) links = sorted([x for x in out.split('\n') if x]) if links: - for l in links: - if self._link_re.search(l) is None: - log.warning("Malformed symlink detected: %s. Skipping.", l) + for link in links: + if self._link_re.search(link) is None: + log.warning("Malformed symlink detected: %s. Skipping.", link) else: - self.exposure(d, l, status) + self.exposure(d, link, status) else: log.warning('No links found, check connection.') # @@ -440,9 +439,9 @@ def backup(self, d, night, status): if self.tape: log.debug(' '.join(cmd)) _, out, err = _popen(cmd) - with open(ls_file) as l: - data = l.read() - backup_files = [l.split()[-1] for l in data.split('\n') if l] + with open(ls_file) as ls_fileobj: + data = ls_fileobj.read() + backup_files = [ls_out.split()[-1] for ls_out in data.split('\n') if ls_out] else: backup_files = [] backup_file = hpss_file + '_' + night + '.tar' @@ -544,9 +543,9 @@ def verify_checksum(checksum_file): checksum_file, n_lines) errors += "{0:d} file(s) listed but not downloaded.\n".format(n_lines) if n_lines < 0: - log.error("%d files are not listed in %s!", -1*n_lines, checksum_file) - errors += "{0:d} file(s) downloaded but not listed.\n".format(-1*n_lines) - digest = dict([(l.split()[1], l.split()[0]) for l in lines if l]) + log.error("%d files are not listed in %s!", -1 * n_lines, checksum_file) + errors += "{0:d} file(s) downloaded but not listed.\n".format(-1 * n_lines) + digest = dict([(cl.split()[1], cl.split()[0]) for cl in lines if cl]) for f in files: ff = os.path.join(d, f) if ff != checksum_file: @@ -668,5 +667,5 @@ def main(): options.kill) return 0 transfer.transfer() - time.sleep(sleep*60) + time.sleep(sleep * 60) return 0 diff --git a/py/desitransfer/daily.py b/py/desitransfer/daily.py index ef67921..c66ec8c 100644 --- a/py/desitransfer/daily.py +++ b/py/desitransfer/daily.py @@ -56,14 +56,14 @@ def transfer(self, permission=True): if self.extra: for i, e in enumerate(self.extra): cmd.insert(cmd.index('--omit-dir-times') + 1 + i, e) - with open(self.log, 'ab') as l: - l.write(("DEBUG: desi_daily_transfer %s\n" % dtVersion).encode('utf-8')) - l.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8')) - l.write(("DEBUG: Transfer start: %s\n" % stamp()).encode('utf-8')) - l.flush() - p = sub.Popen(cmd, stdout=l, stderr=sub.STDOUT) + with open(self.log, 'ab') as logfile: + logfile.write(("DEBUG: desi_daily_transfer %s\n" % dtVersion).encode('utf-8')) + logfile.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8')) + logfile.write(("DEBUG: Transfer start: %s\n" % stamp()).encode('utf-8')) + logfile.flush() + p = sub.Popen(cmd, stdout=logfile, stderr=sub.STDOUT) status = p.wait() - l.write(("DEBUG: Transfer complete: %s\n" % stamp()).encode('utf-8')) + logfile.write(("DEBUG: Transfer complete: %s\n" % stamp()).encode('utf-8')) if status == 0: self.lock() if permission: @@ -80,8 +80,8 @@ def lock(self): fpath = os.path.join(dirpath, f) if stat.S_IMODE(os.stat(fpath).st_mode) != file_perm: os.chmod(fpath, file_perm) - with open(self.log, 'ab') as l: - l.write(("DEBUG: Lock complete: %s\n" % stamp()).encode('utf-8')) + with open(self.log, 'ab') as logfile: + logfile.write(("DEBUG: Lock complete: %s\n" % stamp()).encode('utf-8')) def permission(self): """Set permissions for DESI collaboration access. @@ -95,12 +95,12 @@ def permission(self): The status returned by :command:`fix_permissions.sh`. """ cmd = ['fix_permissions.sh', self.destination] - with open(self.log, 'ab') as l: - l.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8')) - l.flush() - p = sub.Popen(cmd, stdout=l, stderr=sub.STDOUT) + with open(self.log, 'ab') as logfile: + logfile.write(("DEBUG: %s\n" % ' '.join(cmd)).encode('utf-8')) + logfile.flush() + p = sub.Popen(cmd, stdout=logfile, stderr=sub.STDOUT) status = p.wait() - l.write(("DEBUG: Permission reset complete: %s\n" % stamp()).encode('utf-8')) + logfile.write(("DEBUG: Permission reset complete: %s\n" % stamp()).encode('utf-8')) return status diff --git a/py/desitransfer/nightwatch.py b/py/desitransfer/nightwatch.py index e28d4d3..c133623 100644 --- a/py/desitransfer/nightwatch.py +++ b/py/desitransfer/nightwatch.py @@ -114,7 +114,7 @@ def main(): options = _options() _configure_log(options.debug) errcount = 0 - wait = options.sleep*60 + wait = options.sleep * 60 source = '/exposures/nightwatch' basedir = os.path.join(os.environ['DESI_ROOT'], 'spectro', 'nightwatch') kpnodir = os.path.join(basedir, 'kpno') diff --git a/py/desitransfer/test/test_common.py b/py/desitransfer/test/test_common.py index 46f6dd3..f40f0a0 100644 --- a/py/desitransfer/test/test_common.py +++ b/py/desitransfer/test/test_common.py @@ -135,7 +135,7 @@ def test_today(self, mock_dt): """Test today's date. """ mock_dt.datetime.utcnow.return_value = datetime(2019, 7, 3, 5, 0, 0) - mock_dt.timedelta.return_value = timedelta(7/24+0.5) + mock_dt.timedelta.return_value = timedelta(7 / 24 + 0.5) y = today() self.assertEqual(y, '20190702') diff --git a/py/desitransfer/test/test_daemon.py b/py/desitransfer/test/test_daemon.py index e7d41a4..783c121 100644 --- a/py/desitransfer/test/test_daemon.py +++ b/py/desitransfer/test/test_daemon.py @@ -1045,49 +1045,48 @@ def test_verify_checksum(self): d = os.path.dirname(c) with patch('os.listdir') as mock_listdir: mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt'] - with patch('desitransfer.daemon.log') as l: + with patch('desitransfer.daemon.log') as mock_log: o = verify_checksum(c) self.assertEqual(o, "") - l.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt')), - call("%s is valid.", os.path.join(d, 'test_file_2.txt'))]) + mock_log.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt')), + call("%s is valid.", os.path.join(d, 'test_file_2.txt'))]) # # Wrong number of files. # with patch('os.listdir') as mock_listdir: mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt'] - with patch('desitransfer.daemon.log') as l: + with patch('desitransfer.daemon.log') as mock_log: o = verify_checksum(c) self.assertEqual(o, "1 file(s) listed but not downloaded.\n") - l.error.assert_has_calls([call("%s lists %d file(s) that are not present!", c, 1)]) + mock_log.error.assert_has_calls([call("%s lists %d file(s) that are not present!", c, 1)]) with patch('os.listdir') as mock_listdir: mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt', 'test_file_3.txt'] - with patch('desitransfer.daemon.log') as l: + with patch('desitransfer.daemon.log') as mock_log: o = verify_checksum(c) self.assertEqual(o, "1 file(s) downloaded but not listed.\ntest_file_3.txt not listed in checksum file.\n") - l.error.assert_has_calls([call("%d files are not listed in %s!", 1, c)]) + mock_log.error.assert_has_calls([call("%d files are not listed in %s!", 1, c)]) # # Bad list of files. # with patch('os.listdir') as mock_listdir: mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_3.txt'] - with patch('desitransfer.daemon.log') as l: + with patch('desitransfer.daemon.log') as mock_log: o = verify_checksum(c) self.assertEqual(o, "test_file_3.txt not listed in checksum file.\n") - l.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt'))]) - l.error.assert_has_calls([call("%s does not appear in %s!", os.path.join(d, 'test_file_3.txt'), c)]) + mock_log.debug.assert_has_calls([call("%s is valid.", os.path.join(d, 'test_file_1.txt'))]) + mock_log.error.assert_has_calls([call("%s does not appear in %s!", os.path.join(d, 'test_file_3.txt'), c)]) # # Hack hashlib to produce incorrect checksums. # with patch('os.listdir') as mock_listdir: mock_listdir.return_value = ['t.sha256sum', 'test_file_1.txt', 'test_file_2.txt'] - with patch('desitransfer.daemon.log') as l: - with patch('hashlib.sha256') as h: - # h.sha256 = MagicMock() - h.hexdigest.return_value = 'abcdef' + with patch('desitransfer.daemon.log') as mock_log: + with patch('hashlib.sha256') as mock_hash: + mock_hash.hexdigest.return_value = 'abcdef' o = verify_checksum(c) self.assertEqual(o, "test_file_1.txt had a checksum mismatch.\ntest_file_2.txt had a checksum mismatch.\n") - l.error.assert_has_calls([call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_1.txt'), c), - call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_2.txt'), c)]) + mock_log.error.assert_has_calls([call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_1.txt'), c), + call("Checksum mismatch for %s in %s!", os.path.join(d, 'test_file_2.txt'), c)]) @patch('os.walk') @patch('os.chmod') diff --git a/py/desitransfer/test/test_status.py b/py/desitransfer/test/test_status.py index 8f1a0ed..6cfc073 100644 --- a/py/desitransfer/test/test_status.py +++ b/py/desitransfer/test/test_status.py @@ -75,17 +75,17 @@ def test_TransferStatus_init(self): # New directory. # d = '/desi/spectro/status' - with patch('desitransfer.status.log') as l: - with patch('os.makedirs') as m: - with patch('shutil.copy') as cp: - with patch('shutil.copyfile') as cf: + with patch('desitransfer.status.log') as mock_log: + with patch('os.makedirs') as mock_makedirs: + with patch('shutil.copy') as mock_copy: + with patch('shutil.copyfile') as mock_copyfile: s = TransferStatus(d) - l.debug.assert_has_calls([call("os.makedirs('%s', exist_ok=True)", d), - call("shutil.copyfile('%s', '%s')", h, os.path.join(d, 'index.html')), - call("shutil.copy('%s', '%s')", j, d)]) - m.assert_called_once_with(d, exist_ok=True) - cp.assert_called_once_with(j, d) - cf.assert_called_once_with(h, os.path.join(d, 'index.html')) + mock_log.debug.assert_has_calls([call("os.makedirs('%s', exist_ok=True)", d), + call("shutil.copyfile('%s', '%s')", h, os.path.join(d, 'index.html')), + call("shutil.copy('%s', '%s')", j, d)]) + mock_makedirs.assert_called_once_with(d, exist_ok=True) + mock_copy.assert_called_once_with(j, d) + mock_copyfile.assert_called_once_with(h, os.path.join(d, 'index.html')) @patch('desitransfer.status.log') def test_TransferStatus_handle_malformed_with_log(self, mock_log): diff --git a/py/desitransfer/test/test_top_level.py b/py/desitransfer/test/test_top_level.py index 22588b4..8393d93 100644 --- a/py/desitransfer/test/test_top_level.py +++ b/py/desitransfer/test/test_top_level.py @@ -13,8 +13,7 @@ class TestTopLevel(unittest.TestCase): @classmethod def setUpClass(cls): - cls.versionre = re.compile( - r'([0-9]+!)?([0-9]+)(\.[0-9]+)*((a|b|rc|\.post|\.dev)[0-9]+)?') + cls.versionre = re.compile(r'([0-9]+!)?([0-9]+)(\.[0-9]+)*((a|b|rc|\.post|\.dev)[0-9]+)?') @classmethod def tearDownClass(cls): diff --git a/py/desitransfer/tucson.py b/py/desitransfer/tucson.py index 826d601..60524c0 100644 --- a/py/desitransfer/tucson.py +++ b/py/desitransfer/tucson.py @@ -241,7 +241,7 @@ def main(): for s in suffix: if options.sleep.endswith(s): try: - sleepy_time = int(options.sleep[0:-1])*suffix[s] + sleepy_time = int(options.sleep[0:-1]) * suffix[s] except ValueError: log.error("Invalid value for sleep interval: '%s'!", options.sleep) return 1 diff --git a/setup.cfg b/setup.cfg index a5faf6c..3117a46 100644 --- a/setup.cfg +++ b/setup.cfg @@ -115,9 +115,7 @@ exclude_lines = # These are normally ignored by default: # ignore = E121, E123, E126, E133, E226, E241, E242, E704, W503, W504 # -# In addition to the default set we add: +# These are the explicitly ignored styles: # - E501: line too long (82 > 79 characters) -# - E731: do not assign a lambda expression, use a def -# - E741: do not use variables named 'l', 'O', or 'I' -- because, for example, -# 'l' might refer to Galactic longitude. -ignore = E121, E123, E126, E133, E226, E241, E242, E501, E704, E731, E741, W503, W504 +# - W504: line break after binary operator +ignore = E501, W504