Repo review bugbear #9505
Repo review bugbear #9505
Conversation
Armavica
force-pushed
the
repo-checker-pyproject
branch
2 times, most recently
from
767a935 to
5116a3d
Compare
xarray/core/variable.py
Outdated
| @@ -2528,7 +2532,7 @@ def _to_dense(self) -> Variable: | |||
|
|
|||
| def chunk( # type: ignore[override] | |||
| self, | |||
| chunks: T_Chunks = {}, | |||
| chunks: T_Chunks = None, | |||
There was a problem hiding this comment.
IIRC these are intended to be different between {} and None (and the coercion below then doesn't let None through...)
There was a problem hiding this comment.
...though the tests pass, so maybe it's fine (either that or our tests aren't sufficient...)
There was a problem hiding this comment.
Yes, there was a couple of instances where a comment was explaining something like this, so I left these ones alone.
Line 2660 in 1c6300c
xarray/xarray/core/dataarray.py
Line 1363 in 1c6300c
In the absence of such comment (like here) I replaced the default with None, but I could also revert these places and add a comment if it was missing.
There was a problem hiding this comment.
@dcherian to confirm, you would like this:
I could also revert these places and add a comment if it was missing.
| @@ -330,11 +330,13 @@ def test_parents(self): | |||
| def test_lineage(self): | |||
| _, leaf_f = create_test_tree() | |||
| expected = ["f", "e", "b", "a"] | |||
| assert [node.name for node in leaf_f.lineage] == expected | |||
| with pytest.warns(DeprecationWarning): | |||
| assert [node.name for node in leaf_f.lineage] == expected | |||
| @@ -72,7 +73,9 @@ def backends_dict_from_pkg( | |||
| backend = entrypoint.load() | |||
| backend_entrypoints[name] = backend | |||
| except Exception as ex: | |||
| warnings.warn(f"Engine {name!r} loading failed:\n{ex}", RuntimeWarning) | |||
| warnings.warn( | |||
There was a problem hiding this comment.
It would be better to replace all of these with utils.emit_user_level_warning
There was a problem hiding this comment.
(I also think it's fine to do this in another PR, though defer to @dcherian)
We could also add a regex lint in pre-commit that we don't use warnings.warn, we only use utils.emit_level_warning. Here's an example of this from another repo: https://github.com/PRQL/prql/blob/main/.pre-commit-config.yaml#L64-L71
There was a problem hiding this comment.
I also think it's fine to do this in another PR,
yes fine by me!
xarray/testing/strategies.py
Outdated
| @@ -137,7 +139,7 @@ def dimension_names( | |||
|
|
|||
| def dimension_sizes( | |||
| *, | |||
| dim_names: st.SearchStrategy[Hashable] = names(), | |||
| dim_names: st.SearchStrategy[Hashable] | None = None, | |||
There was a problem hiding this comment.
this is unnecessary IIUC, the strategies are immutable.
* B006: mutable argument default * B007: unused loop control variable * B011: assert false
Armavica
force-pushed
the
repo-checker-pyproject
branch
from
8bf1f75 to
6b3c4e9
Compare
* Use ignore instead of extend-ignore for ruff * Fix B028 no explicit stacklevel in warnings.warn * Fix B006, B007 and B011 (auto-fixes) * B006: mutable argument default * B007: unused loop control variable * B011: assert false * Fix the remaining B006 and B007 manually * Fix B015: pointless comparisons * Fix B017: pytest.raises(Exception) * Fix B023: function definition does not bind loop variable * Fix B005: strip with multicharacter strings * Fix B020: loop control variable overrides iterable * Fix B008: no function call in argument defaults * Fix B018: useless expression * Fix B904: raise from within except * Finally enable ruff bugbear lints * Fix readthedocs
resolved conflict stemming from pydata#9505 by accepting this branches changes (pydata#9505 was just a slight edit on the old try import .. except pattern)
Second pass on #8239 mainly addressing bugbear ruff lints.
whats-new.rstapi.rst