-
Notifications
You must be signed in to change notification settings - Fork 12
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
Adopt ruff
as the central tool for linting, formatting, and import sorting
#702
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #702 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 15 15
Lines 1546 1546
=========================================
Hits 1546 1546 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My self-review
@@ -847,7 +847,7 @@ def test_add_monthly_bounds_for_end_of_month_set_to_true(self): | |||
# Get time axis and create new axis with time set to first day of month. | |||
time = ds_with_bnds.time | |||
new_time = [] | |||
for i, t in enumerate(time.values): | |||
for _, t in enumerate(time.values): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -663,7 +668,7 @@ def test_map_latitude_coarse_to_fine(self): | |||
np.testing.assert_allclose(x, y) | |||
|
|||
for x2, y2 in zip(weights, expected_weigths): | |||
np.testing.assert_allclose(x, y) | |||
np.testing.assert_allclose(x2, y2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1254,7 +1259,7 @@ def test_grid(self): | |||
ValueError, | |||
match=r".*lon\d?.*lon\d?.*", | |||
): | |||
ds_multi.regridder.grid | |||
ds_multi.regridder.grid # noqa: B018 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignores B018 - useless-expression
@@ -44,7 +44,7 @@ | |||
def open_dataset( | |||
path: str | os.PathLike[Any] | BufferedIOBase | AbstractDataStore, | |||
data_var: Optional[str] = None, | |||
add_bounds: List[CFAxisKey] | None = ["X", "Y"], | |||
add_bounds: List[CFAxisKey] | Tuple[CFAxisKey, ...] | None = ("X", "Y"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def add_missing_bounds( # noqa: C901 | ||
self, axes: List[CFAxisKey] = ["X", "Y", "T"] | ||
self, axes: List[CFAxisKey] | Tuple[CFAxisKey, ...] = ("X", "Y", "T") | ||
) -> xr.Dataset: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except KeyError as e: | ||
raise RuntimeError( | ||
"Could not determine 'Z' coordinate in output dataset" | ||
) from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except KeyError as e: | ||
if self._target_data is not None and isinstance(self._target_data, str): | ||
raise RuntimeError( | ||
f"Could not find target variable {self._target_data!r} in dataset" | ||
) | ||
) from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except KeyError as e: | ||
raise RuntimeError("Could not determine `Z` coordinate in dataset.") from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except KeyError as e: | ||
raise RuntimeError("Could not determine `Z` bounds in dataset.") from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except (IndexError, ValueError) as e: | ||
raise ValueError( | ||
"Reference periods must be a tuple of strings with the format " | ||
"'yyyy-mm-dd'. For example, reference_period=('1850-01-01', " | ||
"'1899-12-31')." | ||
) | ||
) from e |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4581444
to
545d19b
Compare
@xCDAT/core-developers happy to hear thoughts if any of you care about this like I do. All you need to do on your end is reinstall your conda dev env and |
I trust you. |
Description
flake8
linter,black
formatter, andisort
withruff
#592Why ruff?
flake8
,black
, andisort
flake8
,black
, andisort
out-of-the-boxflake8-docstrings
andflake8-bugbear
Summary of changes
flake8
,black
, andisort
/flake8-isort
with a single tool,ruff
in conda dev environment andpre-commit
ruff
axes=["X", "Y"]
). It is bad practice to set default arguments to a mutable object: https://docs.astral.sh/ruff/rules/mutable-argument-default/#why-is-this-badtuples
should be usedregridder/__init__.py
What do contributors need to do?
conda env create -f conda/dev.yml
xcdat
repo:pre-commit install
pre-commit install
Future TODOs
flake8-docstring
forruff
inpyproject.toml
and fix docstring issuesChecklist
If applicable: