Add support for pre-commit with mypy and Ruff and start adding type hints#173
Add support for pre-commit with mypy and Ruff and start adding type hints#173valeriupredoi merged 52 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (84.62%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #173 +/- ##
=======================================
Coverage 76.83% 76.83%
=======================================
Files 15 15
Lines 2936 2936
Branches 467 467
=======================================
Hits 2256 2256
Misses 558 558
Partials 122 122 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
valeriupredoi
changed the title
|
the failing test is due to netCDF4==1.7.4 and fails in |
|
@davidhassell when you have a minute, let's have this reviewed before it reaches absurd sizes 🍺 |
davidhassell
left a comment
davidhassell
left a comment
There was a problem hiding this comment.
Hi V - looks good. A few minor comments, with which you can do what you like :)
pyfive/high_level.py
Outdated
| """ initalize. """ | ||
| if mode != 'r': | ||
| raise NotImplementedError('pyfive only provides support for reading and treats all reads as binary') | ||
| def __init__(self, filename: str | typing.BinaryIO, mode="r") -> None: |
There was a problem hiding this comment.
Does mode need a type?
There was a problem hiding this comment.
yes indeed, let me pop it in
pyfive/high_level.py
Outdated
|
|
||
| def _get_object_by_address(self, obj_addr): | ||
| """ Return the object pointed to by a given address. """ | ||
| def _get_object_by_address(self, obj_addr: typing.BinaryIO): |
There was a problem hiding this comment.
Missing return type? Or is it OK to omit?
There was a problem hiding this comment.
good catch, let me add one
There was a problem hiding this comment.
added in 5486e7f with an ignore statement too, since the func has inconsistent returns, but they are OK, and mypy shouldn't complain - common false negative
Co-authored-by: David Hassell <davidhassell@users.noreply.github.com>
Co-authored-by: David Hassell <davidhassell@users.noreply.github.com>
Co-authored-by: David Hassell <davidhassell@users.noreply.github.com>
…unc but they should not be changed, this is a common mypy false negative
.github/workflows/pytest.yml
Outdated
| pre-commit install | ||
| pre-commit run -a | ||
| # this will be removed when fully Ruff-ed | ||
| - name: Lint with flake8 |
There was a problem hiding this comment.
@davidhassell I am going to remove linting with flake8 now that we have ruff in, and working, pls holler back if you reckon we should keep it 🍺
|
@davidhassell @bnlawrence @kmuehlbauer I boiled down the issue with netCDF4=1.7.4 to an MRE: import netCDF4 as nc
import numpy as np
# should work as per https://unidata.github.io/netcdf4-python/#netCDF4.stringtochar
months = np.array(["January", "February", "March", "April"], dtype="S8")
print(nc.stringtochar(months))
# but I get:
# Traceback (most recent call last):
# File "/home/valeriu/netcdf4_MRE.py", line 5, in <module>
# print(nc.stringtochar(months))
# ~~~~~~~~~~~~~~~^^^^^^^^
# File "src/netCDF4/_netCDF4.pyx", line 6824, in netCDF4._netCDF4.stringtochar
# AttributeError: 'numpy.bytes_' object has no attribute 'encode'. Did you mean: 'decode'?
# with
# Name Version Build Channel
# netcdf4 1.7.4 nompi_py314h4ae7121_101 conda-forge
# everything OK with netCDF4==1.7.3stringtochar is FUBAR, am going to open an issue at netCDF4 |
|
got this one open Unidata/netcdf4-python#1464 and I am pinning to |
|
great many thanks for the review @davidhassell - we're all set 🍻 |
2 participants
Description
OK @bnlawrence @davidhassell I have added full infrastructure for pre-commit with ruff and mypy
To run pre-commit, check out this branch, make sure you have pre-commit installed (from conda-forge) then:
Contributes twoards #168
(closing upon reviewer's discretion, not via merge from here)
Closes #121
Before you get started
Checklist