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

Enable Ruff linting rules (Bugbear, Pyupgrade, Ruff...) #214

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

VeckoTheGecko
Copy link
Contributor

@VeckoTheGecko VeckoTheGecko commented Dec 20, 2024

Enable the following Ruff linting rules and fix associated errors. See docs for more info on the available rules:

  • B (Bugbear)
  • UP (pyupgrade)
  • LOG (logging)
  • ICN (import conventions)
  • G (logging-format)
  • RUF (ruff)
    See pyproject.toml for full config changes.

The linting rules do static analysis to bring things in line with modern Python best practices (according to the version of Python being used in the pyproject.toml). Some of these items are autofixed, while other times just flagged by the linter.

Note that "best practices" ranges from objective to subjective.

  • Objective: Stuff like UP009 removing # -*- coding: utf-8 -*- from the top of files is objectively good, as Python 3 files are all utf8 encoded, so there's no need to flag the encoding.
  • subjective: sorting __all__

Rules can be ignored/unselected at the discretion of maintainers.

See a list of the items that were flagged below, their the locations in the codebase, and associated rules.

pre-commit log

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check python ast.........................................................Passed
check json...........................................(no files to check)Skipped
ruff lint................................................................Failed
- hook id: ruff
- exit code: 1
- files were modified by this hook

pyglider/ncprocess.py:186:40: UP031 Use format specifiers instead of percent format
    |
184 |                 # add traj_strlen using bare ntcdf to make IOOS happy
185 |                 with netCDF4.Dataset(outname, 'r+') as nc:
186 |                     nc.renameDimension('string%d' % trajlen, 'traj_strlen')
    |                                        ^^^^^^^^^^ UP031
    |
    = help: Replace with format specifiers

pyglider/seaexplorer.py:23:14: UP031 Use format specifiers instead of percent format
   |
21 |     fns = fnout.split('.')
22 |     fns = fns[:5]
23 |     fns[4] = '%04d' % int(fns[4])
   |              ^^^^^^ UP031
24 |     fns[1] = '%04d' % int(fns[1])
25 |     fnout = ''
   |
   = help: Replace with format specifiers

pyglider/seaexplorer.py:24:14: UP031 Use format specifiers instead of percent format
   |
22 |     fns = fns[:5]
23 |     fns[4] = '%04d' % int(fns[4])
24 |     fns[1] = '%04d' % int(fns[1])
   |              ^^^^^^ UP031
25 |     fnout = ''
26 |     for ff in fns:
   |
   = help: Replace with format specifiers

pyglider/seaexplorer.py:130:17: B007 Loop control variable `ind` not used within loop body
    |
128 |                 continue
129 | 
130 |             for ind, f in enumerate(files):
    |                 ^^^ B007
131 |                 # output name:
132 |                 fnout, filenum = _outputname(f, outdir)
    |
    = help: Rename unused `ind` to `_ind`

pyglider/slocum.py:163:9: B007 Loop control variable `i` not used within loop body
    |
161 |     outlines = []
162 |     sensorInfo = {}
163 |     for i in range(nsensors_total):
    |         ^ B007
164 |         line = dfh.readline().decode('utf-8')
165 |         if line.split(':')[0] != 's':
    |
    = help: Rename unused `i` to `_i`

pyglider/slocum.py:272:17: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
    |
270 |                   _make_cache(outlines, cachedir, meta)
271 |               else:
272 |                   raise FileNotFoundError(
    |  _________________^
273 | |                     'No active sensor list found for crc ',
274 | |                     f'{meta["sensor_list_crc"]}. These are often found in ',
275 | |                     'offloaddir/Science/STATE/CACHE/ or ',
276 | |                     'offloaddir/Main_board/STATE/CACHE/. ',
277 | |                     f'Copy those locally into {cachedir}',
278 | |                 )
    | |_________________^ B904
279 |           meta['activeSensorList'] = activeSensorList
280 |           # get the file's timestamp...
    |

pyglider/slocum.py:444:9: UP031 Use format specifiers instead of percent format
    |
442 |       proctimeend = time.time()
443 |       _log.info(
444 |           ('%s lines of data read from %s, data rate of %s rows ' 'per second')
    |  _________^
445 | |         % (len(data), dinkum_file, len(data) / (proctimeend - proctimestart))
    | |_____________________________________________________________________________^ UP031
446 |       )
447 |       dfh.close()
    |
    = help: Replace with format specifiers

pyglider/slocum.py:608:23: UP031 Use format specifiers instead of percent format
    |
606 |         listst = ''
607 |         for sensor in meta['activeSensorList']:
608 |             listst += '%s' % sensor
    |                       ^^^^^^^^^^^^^ UP031
609 |             listst += '\n'
610 |         ds.attrs['activeSensorList'] = listst
    |
    = help: Replace with format specifiers

pyglider/utils.py:127:12: B007 Loop control variable `i` not used within loop body
    |
125 |     dpall = np.diff(p)
126 |     inflect = np.where(dpall[:-1] * dpall[1:] < 0)[0]
127 |     for n, i in enumerate(inflect[:-1]):
    |            ^ B007
128 |         nprofile = inflect[n + 1] - inflect[n]
129 |         inds = np.arange(good[inflect[n]], good[inflect[n + 1]] + 1) + 1
    |
    = help: Rename unused `i` to `_i`

pyglider/utils.py:556:39: UP031 Use format specifiers instead of percent format
    |
555 |     dt = ds.time.values
556 |     ds.attrs['time_coverage_start'] = '%s' % dt[0]
    |                                       ^^^^^^^^^^^^ UP031
557 |     ds.attrs['time_coverage_end'] = '%s' % dt[-1]
    |
    = help: Replace with format specifiers

pyglider/utils.py:557:37: UP031 Use format specifiers instead of percent format
    |
555 |     dt = ds.time.values
556 |     ds.attrs['time_coverage_start'] = '%s' % dt[0]
557 |     ds.attrs['time_coverage_end'] = '%s' % dt[-1]
    |                                     ^^^^^^^^^^^^^ UP031
558 | 
559 |     ds.attrs['processing_level'] = (
    |
    = help: Replace with format specifiers

pyglider/utils.py:754:12: B006 Do not use mutable data structures for argument defaults
    |
752 |     filename,
753 |     outname,
754 |     toplot=['potential_temperature', 'salinity', 'oxygen_concentration'],
    |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ B006
755 |     pdenlevels=np.arange(10, 30, 0.5),
756 |     dpi=200,
    |
    = help: Replace with `None`; initialize within function

pyglider/utils.py:755:16: B008 Do not perform function call `np.arange` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
    |
753 |     outname,
754 |     toplot=['potential_temperature', 'salinity', 'oxygen_concentration'],
755 |     pdenlevels=np.arange(10, 30, 0.5),
    |                ^^^^^^^^^^^^^^^^^^^^^^ B008
756 |     dpi=200,
757 |     ylim=None,
    |

pyglider/utils.py:793:9: B007 Loop control variable `nn` not used within loop body
    |
791 |         ]
792 |     deployment = {}
793 |     for nn, d in enumerate(deploymentyaml):
    |         ^^ B007
794 |         with open(d) as fin:
795 |             deployment_ = yaml.safe_load(fin)
    |
    = help: Rename unused `nn` to `_nn`

tests/test_seaexplorer.py:13:1: E402 Module level import not at top of file
   |
11 | example_dir = library_dir / 'tests/example-data/'
12 | 
13 | import pyglider.seaexplorer as seaexplorer
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ E402
   |


Fixed 11 errors:
- pyglider/__init__.py:
    1 × UP009 (utf8-encoding-declaration)
- pyglider/seaexplorer.py:
    1 × UP009 (utf8-encoding-declaration)
    1 × RUF022 (unsorted-dunder-all)
- pyglider/slocum.py:
    2 × UP015 (redundant-open-modes)
    1 × RUF022 (unsorted-dunder-all)
- pyglider/utils.py:
    2 × UP015 (redundant-open-modes)
    1 × RUF022 (unsorted-dunder-all)
- tests/test_slocum.py:
    2 × UP015 (redundant-open-modes)

Found 26 errors (11 fixed, 15 remaining).
No fixes available (9 hidden fixes can be enabled with the `--unsafe-fixes` option).

ruff-format..............................................................Passed
prettier.................................................................Passed
taplo-format.............................................................Passed
Validate pyproject.toml..................................................Passed

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 57.89474% with 8 lines in your changes missing coverage. Please review.

Project coverage is 54.90%. Comparing base (85011e5) to head (e5e5132).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pyglider/slocum.py 0.00% 5 Missing ⚠️
pyglider/utils.py 50.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #214   +/-   ##
=======================================
  Coverage   54.90%   54.90%           
=======================================
  Files           9        9           
  Lines        1692     1692           
=======================================
  Hits          929      929           
  Misses        763      763           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@VeckoTheGecko VeckoTheGecko marked this pull request as ready for review December 20, 2024 09:36
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.

1 participant