Replace black, isort, flake8 with ruff #446

sverhoeven · 2024-07-24T12:46:49Z

Refs #441

TODO

TODO * [ ] valid pyproject.toml for `pip install -e .[dev]`

Currently pip does not understand pyproject.toml file

Skipped flake8 config as this will be replaced by ruff

…ymore

Unable to exclude during lock generation as `--no-dev-dependencies` moves dev deps to main cat.

Because it is imported in this package

This reverts commit 0bc3f89. Was test script not part of repo

Used unsafe fixes to fix

Adhering to FBT rules require lots of refactoring and changes to public api

That we are compliant with

Comment out rules that need lot of refactoring

BSchilperoort

Nice work, these changes do increase the quality of the code base!

I only encountered some minor issues, after those are fixed feel free to merge 👍

BSchilperoort · 2024-07-29T12:48:14Z

src/ewatercycle/_forcings/caravan.py

-            with http.request(
-                "GET", SHAPEFILE_URL, preload_content=False
-            ) as r, zip_path.open("wb") as out_file:
+            with (


Using parentheses with a multi-line context manager has finally been available since Python 3.10. Makes writing tests with mocks easier to read too. Nice to see that the ruff formatter/linter caught this.

BSchilperoort · 2024-07-29T12:50:31Z

src/ewatercycle/base/__init__.py

Wish we could remove these __init__ files (not needed anymore according to the PEPs), but Sphinx still needs it...

src/ewatercycle/base/forcing.py

src/ewatercycle/base/model.py

src/ewatercycle/container.py

BSchilperoort · 2024-07-29T13:54:39Z

src/ewatercycle/container.py

@@ -60,6 +66,8 @@ class ContainerImage(str):
    eWatercycle containers typically don't have these issues.
    """

+    __slots__ = ()


Why did you decide to add __slots__ here?

Because https://docs.astral.sh/ruff/rules/no-slots-in-str-subclass/ told me to.

BSchilperoort · 2024-07-29T14:17:42Z

src/ewatercycle/esmvaltool/run.py

    forcing_files = {}
    for output_file in output_files:
        var_name = output_file.path.stem
        if isinstance(output_file, DataFile):
            # Datafile means ends with .nc
            # Use first variable name from inside file as key
            dataset = output_file.load_xarray()
-            var_name = list(dataset.data_vars.keys())[0]
+            var_name = next(iter(dataset.data_vars.keys()))


Why did you not noqa this line, but you did on line 157.

(first_diagnostic_output = list(recipe_output.values())[0] # noqa: RUF015)

Because the ruff fix caused test to fail, so I reverted to orignal

`______________________________________________________________________ test_parse_recipe_output_with_no_diagnostic _______________________________________________________________________ def test_parse_recipe_output_with_no_diagnostic(): recipe_output = RecipeOutput({}, info=RecipeInfo({"diagnostics": {}}, "script")) with pytest.raises(IndexError): > _parse_recipe_output(recipe_output) tests/src/esmvaltool/test_run.py:53: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ recipe_output = def _parse_recipe_output(recipe_output: RecipeOutput) -> dict[str, str]: """Parse ESMValTool recipe output into a dictionary. This method assumes: * Recipe had at least one diagnostic * Diagnostic produced at least one file * All files are in the same directory * The first variable name in a NetCDF file is the primary one Returns: Dictionary with forcing data variables as keys and file names as values and a key called directory with value the parent directory of the file names. """ > first_diagnostic_output = next(iter(recipe_output.values())) E StopIteration src/ewatercycle/esmvaltool/run.py:157: StopIteration The above exception was the direct cause of the following exception: cls = <class '_pytest.runner.CallInfo'>, func = <function call_and_report.<locals>.<lambda> at 0x743f92faab60>, when = 'call' reraise = (<class '_pytest.outcomes.Exit'>, <class 'KeyboardInterrupt'>) @classmethod def from_call( cls, func: Callable[[], TResult], when: Literal["collect", "setup", "call", "teardown"], reraise: type[BaseException] | tuple[type[BaseException], ...] | None = None, ) -> CallInfo[TResult]: """Call func, wrapping the result in a CallInfo. :param func: The function to call. Called without arguments. :type func: Callable[[], _pytest.runner.TResult] :param when: The phase in which the function is called. :param reraise: Exception or exceptions that shall propagate if raised by the function, instead of being wrapped in the CallInfo. """ excinfo = None start = timing.time() precise_start = timing.perf_counter() try: > result: TResult | None = func() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/runner.py:341: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/runner.py:242: in <lambda> lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/pluggy/_hooks.py:513: in __call__ return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult) ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/pluggy/_manager.py:120: in _hookexec return self._inner_hookexec(hook_name, methods, kwargs, firstresult) ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/threadexception.py:92: in pytest_runtest_call yield from thread_exception_runtest_hook() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/threadexception.py:68: in thread_exception_runtest_hook yield ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/unraisableexception.py:95: in pytest_runtest_call yield from unraisable_exception_runtest_hook() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/unraisableexception.py:70: in unraisable_exception_runtest_hook yield ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/logging.py:848: in pytest_runtest_call yield from self._runtest_for(item, "call") ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/logging.py:831: in _runtest_for yield _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <CaptureManager _method='fd' _global_capturing=<MultiCapture out=<FDCapture 1 oldfd=6 _state='suspended' tmpfile=<_io....xtIOWrapper name='/dev/null' mode='r' encoding='utf-8'>> _state='suspended' _in_suspended=False> _capture_fixture=None> item = <Function test_parse_recipe_output_with_no_diagnostic> @hookimpl(wrapper=True) def pytest_runtest_call(self, item: Item) -> Generator[None, None, None]: with self.item_capture("call", item): > return (yield) E RuntimeError: generator raised StopIteration ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/capture.py:879: RuntimeError`

BSchilperoort · 2024-07-29T14:22:40Z

src/ewatercycle/observation/caravan.py

@@ -56,7 +55,8 @@ def get_caravan_data(
            lon         float64 8B ...
            area        float64 8B ...
            streamflow  (time) float32 12B ...
-        Attributes:
+
+    Attributes:


Suggested change

Attributes:

Attributes:

This is the output of xarray, but is misidentified by the formatter? It should stay indented.

This was probably result of ruff check --fix --unsafe-fixes

To fix I had to noqa E501, D410, D411 and D214

src/ewatercycle/observation/grdc.py

Alternatives like EWaterCycleModel or BaseModel are worse.

Not a section in the docstring

sverhoeven

Thanks for reviewing.

After your review I added back the precommit to ci.

sverhoeven · 2024-07-31T07:28:19Z

src/ewatercycle/_forcings/caravan.py

-            with http.request(
-                "GET", SHAPEFILE_URL, preload_content=False
-            ) as r, zip_path.open("wb") as out_file:
+            with (


src/ewatercycle/base/forcing.py

src/ewatercycle/base/model.py

src/ewatercycle/container.py

sverhoeven · 2024-07-31T08:06:02Z

src/ewatercycle/container.py

@@ -60,6 +66,8 @@ class ContainerImage(str):
    eWatercycle containers typically don't have these issues.
    """

+    __slots__ = ()


Because https://docs.astral.sh/ruff/rules/no-slots-in-str-subclass/ told me to.

sverhoeven · 2024-07-31T08:09:35Z

src/ewatercycle/esmvaltool/run.py

    forcing_files = {}
    for output_file in output_files:
        var_name = output_file.path.stem
        if isinstance(output_file, DataFile):
            # Datafile means ends with .nc
            # Use first variable name from inside file as key
            dataset = output_file.load_xarray()
-            var_name = list(dataset.data_vars.keys())[0]
+            var_name = next(iter(dataset.data_vars.keys()))


Because the ruff fix caused test to fail, so I reverted to orignal

`______________________________________________________________________ test_parse_recipe_output_with_no_diagnostic _______________________________________________________________________ def test_parse_recipe_output_with_no_diagnostic(): recipe_output = RecipeOutput({}, info=RecipeInfo({"diagnostics": {}}, "script")) with pytest.raises(IndexError): > _parse_recipe_output(recipe_output) tests/src/esmvaltool/test_run.py:53: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ recipe_output = def _parse_recipe_output(recipe_output: RecipeOutput) -> dict[str, str]: """Parse ESMValTool recipe output into a dictionary. This method assumes: * Recipe had at least one diagnostic * Diagnostic produced at least one file * All files are in the same directory * The first variable name in a NetCDF file is the primary one Returns: Dictionary with forcing data variables as keys and file names as values and a key called directory with value the parent directory of the file names. """ > first_diagnostic_output = next(iter(recipe_output.values())) E StopIteration src/ewatercycle/esmvaltool/run.py:157: StopIteration The above exception was the direct cause of the following exception: cls = <class '_pytest.runner.CallInfo'>, func = <function call_and_report.<locals>.<lambda> at 0x743f92faab60>, when = 'call' reraise = (<class '_pytest.outcomes.Exit'>, <class 'KeyboardInterrupt'>) @classmethod def from_call( cls, func: Callable[[], TResult], when: Literal["collect", "setup", "call", "teardown"], reraise: type[BaseException] | tuple[type[BaseException], ...] | None = None, ) -> CallInfo[TResult]: """Call func, wrapping the result in a CallInfo. :param func: The function to call. Called without arguments. :type func: Callable[[], _pytest.runner.TResult] :param when: The phase in which the function is called. :param reraise: Exception or exceptions that shall propagate if raised by the function, instead of being wrapped in the CallInfo. """ excinfo = None start = timing.time() precise_start = timing.perf_counter() try: > result: TResult | None = func() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/runner.py:341: _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/runner.py:242: in <lambda> lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/pluggy/_hooks.py:513: in __call__ return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult) ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/pluggy/_manager.py:120: in _hookexec return self._inner_hookexec(hook_name, methods, kwargs, firstresult) ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/threadexception.py:92: in pytest_runtest_call yield from thread_exception_runtest_hook() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/threadexception.py:68: in thread_exception_runtest_hook yield ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/unraisableexception.py:95: in pytest_runtest_call yield from unraisable_exception_runtest_hook() ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/unraisableexception.py:70: in unraisable_exception_runtest_hook yield ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/logging.py:848: in pytest_runtest_call yield from self._runtest_for(item, "call") ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/logging.py:831: in _runtest_for yield _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ self = <CaptureManager _method='fd' _global_capturing=<MultiCapture out=<FDCapture 1 oldfd=6 _state='suspended' tmpfile=<_io....xtIOWrapper name='/dev/null' mode='r' encoding='utf-8'>> _state='suspended' _in_suspended=False> _capture_fixture=None> item = <Function test_parse_recipe_output_with_no_diagnostic> @hookimpl(wrapper=True) def pytest_runtest_call(self, item: Item) -> Generator[None, None, None]: with self.item_capture("call", item): > return (yield) E RuntimeError: generator raised StopIteration ../../../miniforge3/envs/ewatercycle/lib/python3.12/site-packages/_pytest/capture.py:879: RuntimeError`

sverhoeven · 2024-07-31T08:21:25Z

src/ewatercycle/observation/caravan.py

@@ -56,7 +55,8 @@ def get_caravan_data(
            lon         float64 8B ...
            area        float64 8B ...
            streamflow  (time) float32 12B ...
-        Attributes:
+
+    Attributes:


This was probably result of ruff check --fix --unsafe-fixes

To fix I had to noqa E501, D410, D411 and D214

sonarcloud · 2024-07-31T08:55:42Z

Quality Gate failed

Failed conditions
69.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

sverhoeven and others added 30 commits July 23, 2024 13:30

Replace environment.yml with conda-lock.yml

2ddeee1

TODO * [ ] valid pyproject.toml for `pip install -e .[dev]`

Configure conda-lock in pyproject.toml

5591c14

Currently pip does not understand pyproject.toml file

Switched to hatch + deleted unneeded setup.cfg and setup.py

efc4066

Make pyproject.toml valid + add sections not migrated by hatch

9ba50b6

Skipped flake8 config as this will be replaced by ruff

Merge branch 'hatch' into lock

e75820d

Remove esmvaltool as python dep

ce0ed7e

Remove docs and test from sdist + mypy stricter and fix

f401970

Merge remote-tracking branch 'origin/hatch' into lock

9468c26

Move mypy from precommit to ci + remove pyroma as it is not useful an…

e99deb4

…ymore

Move mypy from precommit to ci + remove pyroma as it is not useful an…

6746f7f

…ymore

Use lock in ci

a521171

Add env name to ci

56e12ed

Use shell for mypy

1815c9f

Use shell for mypy

71d0eb0

Merge branch 'hatch' into lock

03a6abb

Exclude lock from lint

80a80f9

Merge remote-tracking branch 'origin/main' into lock

1c8876c

Use lock in docs

67d91f8

Typo

661ddfe

No longer need MANIFEST.in

cfbe9ff

Exclude dev deps during install

a400dc5

Unable to exclude during lock generation as `--no-dev-dependencies` moves dev deps to main cat.

Merge remote-tracking branch 'origin/main' into lock

0f7910c

Use lock file to install dev deps

42bc30f

Remove unused dependencies

e9470b9

Add osx-64 + lock grpc4bmi deps

6847038

Regenerate lock file

6272453

Add missing fsspec dep

6a127f5

changelog entry

af51dfe

Add cartopy as dep

127ca02

Because it is imported in this package

Give nicer error when esmvaltool/esmvalcore package can not be found

b1f9083

Revert "Add scipy to dev deps as a test imports it"

1f23ee4

This reverts commit 0bc3f89. Was test script not part of repo

Base automatically changed from nobump to main July 29, 2024 08:36

sverhoeven added 16 commits July 29, 2024 10:43

Merge remote-tracking branch 'origin/main' into ruff

b20ab98

Add flake8-builtins ruff rules

dca86aa

Add flake8-unused-arguments

f06a696

Add flake8-bugbear

010b387

Add C rules

7f7aa6f

Add pydocstyle

6a915be

Make mypy and ruff happy

077c362

Add all pydocstyle rules

c2cdb26

Add flake8-datetimez

777a8b2

Add flake8-errmsg rules

a206347

Used unsafe fixes to fix

Enable ruff specific rules

8980dcf

Leaving FBT for later

3953fb4

Adhering to FBT rules require lots of refactoring and changes to public api

Add isort rules

940f3fa

flake8-import-conventions + flake8-implicit-str-concat + pep8-naming

08c40e9

Add bunch more rules

477295c

That we are compliant with

Enable more rules

17ebafd

Comment out rules that need lot of refactoring

sverhoeven marked this pull request as ready for review July 29, 2024 12:33

sverhoeven requested a review from BSchilperoort July 29, 2024 12:34

BSchilperoort approved these changes Jul 29, 2024

View reviewed changes

sverhoeven added 5 commits July 31, 2024 10:01

We are OK with eWaterCycleModel class name

1ee2bda

Alternatives like EWaterCycleModel or BaseModel are worse.

Remove duplicate noqa

342e467

Attributes are part of xarray output

3f74d38

Not a section in the docstring

Run non-ruff precommit hooks on ci

3d8b836

Undo RUF015 fix, does not work in test

74a9154

sverhoeven commented Jul 31, 2024

View reviewed changes

sverhoeven merged commit ed52fbe into main Jul 31, 2024
3 of 4 checks passed

sverhoeven deleted the ruff branch July 31, 2024 08:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace black, isort, flake8 with ruff #446

Replace black, isort, flake8 with ruff #446

sverhoeven commented Jul 24, 2024 •

edited

Loading

BSchilperoort left a comment

BSchilperoort Jul 29, 2024

sverhoeven Jul 31, 2024

BSchilperoort Jul 29, 2024 •

edited

Loading

BSchilperoort Jul 29, 2024

sverhoeven Jul 31, 2024

BSchilperoort Jul 29, 2024

sverhoeven Jul 31, 2024

BSchilperoort Jul 29, 2024

sverhoeven Jul 31, 2024

sverhoeven left a comment

sverhoeven Jul 31, 2024

sverhoeven Jul 31, 2024

sverhoeven Jul 31, 2024

sverhoeven Jul 31, 2024

sonarcloud bot commented Jul 31, 2024

Replace black, isort, flake8 with ruff #446

Replace black, isort, flake8 with ruff #446

Conversation

sverhoeven commented Jul 24, 2024 • edited Loading

BSchilperoort left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BSchilperoort Jul 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sverhoeven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 31, 2024

Quality Gate failed

sverhoeven commented Jul 24, 2024 •

edited

Loading

BSchilperoort Jul 29, 2024 •

edited

Loading