Enforce ruff/flake8-simplify rules (SIM)

[DimitriPapadopoulos]

• Closes #xxxx
• Tests added
• User visible changes (including notable bug fixes) are documented in whats-new.rst
• New functions/methods are listed in api.rst

[DimitriPapadopoulos]

I don't see how this change causes the new mypy errors:

Found 2 errors in 1 file (checked 191 source files)
xarray/core/dataset.py:8986: error: Value expression in dictionary comprehension has incompatible type "float | tuple[float, float] | Mapping[Any, float | tuple[float, float]]"; expected type "float | tuple[float, float]" [misc]
xarray/core/dataset.py:8991: error: Incompatible types in assignment (expression has type "float | tuple[float, float] | Mapping[Any, float | tuple[float, float]] | Mapping[Any, float | tuple[float, float] | Mapping[Any, float | tuple[float, float]]] | None", variable has type "float | tuple[float, float] | Mapping[Any, float | tuple[float, float]]") [assignment]

Wasn't this a pre-existing issue? If so, not sure why mypy only flags it after this change.

[max-sixty]

yes agree it's confusing...

[keewis]

it looks like there's a SIM118 error on main now. I'd argue that we should not accept that particular one (iterating over a dataset feels confusing to me: are we iterating over all variables, coordinates, or data vars?), or if we do we should access the appropriate property to be explicit about what would happen (i.e. access .data_vars, .coords, or .variables)

[max-sixty]

sorry re error, I'll revert. (but why no error in the tests?)

no strong view re datasets; I was partial to the proposed code but no strong view

[keewis]

no idea why it didn't fail here.

to be clear, the difference is between iterating over self.ds or self.ds.keys(). Dataset is dict-like, but it's usually not clear what we iterate over by default (I think data variables only?). So while we can for sure omit the .keys(), I'd argue self.ds.data_vars would be much easier to understand (or similar, should I have forgotten the behavior of ds.keys()).

Edit: thanks for reverting, although I think fixing the failing check in a new PR would have also worked

[DimitriPapadopoulos]

My wrong, if it's a dataset, obviously keeping keys() is better. Besides, the SIM rules are more error-prone, that's why they're often unsafe rules with no auto fixes. Would you like to ignore rule SIM118 globally or use noqa on a case by case basis?

[max-sixty]

I would probably go by rule rather than adding noqas but no strong view!

[DimitriPapadopoulos]

I would rather ignore globally too, see #10480 (this time without SIM118).