From 73eebd5d75b57cbbc6a2451fc882b285a7d0ac01 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 18:48:54 -0700 Subject: [PATCH 1/8] Raise an error on new warnings from within xarray --- pyproject.toml | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 0fc26e5b82e..cf1ec2abf71 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -282,9 +282,50 @@ ban-relative-imports = "all" [tool.pytest.ini_options] addopts = ["--strict-config", "--strict-markers"] + +# We want to forbid warnings from within xarray in our tests — instead we should +# fix our own code, or mark the test itself as expecting a warning. So this: +# - Converts any warning from xarray into an error +# - Ignores warnings we already have; since it wasn't practical to fix them all +# before merging this config. We can remove these over time -- if you see a +# warning that we now handle, remove the line. If you are up for a small +# contribution, find the warning and fix it! The warnings are still reported +# in CI. +# +# While we only raise an error on warnings from within xarray, if an dependency +# raises with a stacklevel such that it looks like it raises from xarray, it's +# very acceptable to ignore it here. +# +# If this gets in the way of making progress, it's also acceptable to +# temporarily add additional ignores. + filterwarnings = [ - "ignore:Using a non-tuple sequence for multidimensional indexing is deprecated:FutureWarning", + "error:::xarray.*", + "default::UserWarning:xarray.tests.test_coding_times", + "default::UserWarning:xarray.tests.test_computation", + "default::UserWarning:xarray.tests.test_dataset", + "default:`ancestors` has been deprecated:DeprecationWarning:xarray.core.treenode", + "default:`iter_lineage` has been deprecated:DeprecationWarning:xarray.core.treenode", + "default:`lineage` has been deprecated:DeprecationWarning:xarray.core.treenode", + "default:coords should be an ndarray:DeprecationWarning:xarray.tests.test_variable", + "default:deallocating CachingFileManager:RuntimeWarning:xarray.backends.*", + "default:deallocating CachingFileManager:RuntimeWarning:xarray.backends.netCDF4_", + "default:deallocating CachingFileManager:RuntimeWarning:xarray.core.indexing", + "default:Failed to decode variable.*NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays:DeprecationWarning", + "default:dropping variables using `drop` is deprecated; use drop_vars:DeprecationWarning:xarray.tests.test_groupby", + "default:The `interpolation` argument to quantile was renamed to `method`:FutureWarning:xarray.*", + "default:invalid value encountered in cast:RuntimeWarning:xarray.core.duck_array_ops", + "default:invalid value encountered in cast:RuntimeWarning:xarray.conventions", + "default:invalid value encountered in cast:RuntimeWarning:xarray.tests.test_units", + "default:invalid value encountered in cast:RuntimeWarning:xarray.tests.test_array_api", + "default:NumPy will stop allowing conversion of:DeprecationWarning", + "default:shape should be provided:DeprecationWarning:xarray.tests.test_variable", + "default:the `pandas.MultiIndex` object:FutureWarning:xarray.tests.test_variable", + "default:Using a non-tuple sequence for multidimensional indexing is deprecated:FutureWarning", + "default:Duplicate dimension names present:UserWarning:xarray.namedarray.core", + "default:::xarray.tests.test_strategies", ] + log_cli_level = "INFO" markers = [ "flaky: flaky tests", From ccc5cb44940e9da4f4abf8102208259ef105c77d Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 19:09:16 -0700 Subject: [PATCH 2/8] --- .github/workflows/ci.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 7e097fa5c45..362b6ea13f2 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -85,6 +85,10 @@ jobs: else echo "CONDA_ENV_FILE=ci/requirements/${{ matrix.env }}.yml" >> $GITHUB_ENV fi + elif [[ "${{ matrix.env }}" == "min-all-deps" ]] ; + then + # Ignore any errors + echo "PYTEST_EXTRA_FLAGS=-W default" >> $GITHUB_ENV else if [[ ${{ matrix.python-version }} != "3.12" ]]; then echo "CONDA_ENV_FILE=ci/requirements/environment.yml" >> $GITHUB_ENV From e348383814a6f54babf23da9d94414efc9906358 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 20:19:44 -0700 Subject: [PATCH 3/8] --- .github/workflows/ci.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 362b6ea13f2..0114c6a8a53 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -81,7 +81,7 @@ jobs: if [[ "${{ matrix.env }}" == "flaky" ]] ; then echo "CONDA_ENV_FILE=ci/requirements/environment.yml" >> $GITHUB_ENV - echo "PYTEST_EXTRA_FLAGS=--run-flaky --run-network-tests" >> $GITHUB_ENV + echo "PYTEST_EXTRA_FLAGS=--run-flaky --run-network-tests --W default" >> $GITHUB_ENV else echo "CONDA_ENV_FILE=ci/requirements/${{ matrix.env }}.yml" >> $GITHUB_ENV fi @@ -139,6 +139,8 @@ jobs: enableCrossOsArchive: true save-always: true + - run: echo $PYTEST_EXTRA_FLAGS + - name: Run tests run: python -m pytest -n 4 --timeout 180 From 026235d04e41526a5d016435b1f04516d487c01c Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 20:27:30 -0700 Subject: [PATCH 4/8] --- .github/workflows/ci.yaml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 0114c6a8a53..c26c4fcebf7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -81,14 +81,15 @@ jobs: if [[ "${{ matrix.env }}" == "flaky" ]] ; then echo "CONDA_ENV_FILE=ci/requirements/environment.yml" >> $GITHUB_ENV - echo "PYTEST_EXTRA_FLAGS=--run-flaky --run-network-tests --W default" >> $GITHUB_ENV + echo "PYTEST_EXTRA_FLAGS=--run-flaky --run-network-tests -W default" >> $GITHUB_ENV else echo "CONDA_ENV_FILE=ci/requirements/${{ matrix.env }}.yml" >> $GITHUB_ENV fi - elif [[ "${{ matrix.env }}" == "min-all-deps" ]] ; - then - # Ignore any errors - echo "PYTEST_EXTRA_FLAGS=-W default" >> $GITHUB_ENV + if [[ "${{ matrix.env }}" == "min-all-deps" ]] ; + then + # Don't raise on warnings + echo "PYTEST_EXTRA_FLAGS=-W default" >> $GITHUB_ENV + fi else if [[ ${{ matrix.python-version }} != "3.12" ]]; then echo "CONDA_ENV_FILE=ci/requirements/environment.yml" >> $GITHUB_ENV From a1ce373c55d9d2f246d31ccc9b59a02692a3486c Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 20:30:32 -0700 Subject: [PATCH 5/8] --- .github/workflows/ci.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c26c4fcebf7..e970bc8efc3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -140,8 +140,6 @@ jobs: enableCrossOsArchive: true save-always: true - - run: echo $PYTEST_EXTRA_FLAGS - - name: Run tests run: python -m pytest -n 4 --timeout 180 From bd4b64a7e8e804eebc94df44b6b26baa7e6637f2 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Wed, 24 Apr 2024 20:33:16 -0700 Subject: [PATCH 6/8] --- pyproject.toml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index cf1ec2abf71..0cd61e84b82 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -286,15 +286,15 @@ addopts = ["--strict-config", "--strict-markers"] # We want to forbid warnings from within xarray in our tests — instead we should # fix our own code, or mark the test itself as expecting a warning. So this: # - Converts any warning from xarray into an error -# - Ignores warnings we already have; since it wasn't practical to fix them all -# before merging this config. We can remove these over time -- if you see a -# warning that we now handle, remove the line. If you are up for a small -# contribution, find the warning and fix it! The warnings are still reported -# in CI. +# - Ignores warnings which the test suite currently raises; since it wasn't +# - practical to fix them all before merging this config. We can remove these +# over time -- if you see a warning that we now handle, remove the exception. +# If you are up for a small contribution, find the warning and fix it! The +# warnings are still listed (`default`, not `ignore`). # -# While we only raise an error on warnings from within xarray, if an dependency -# raises with a stacklevel such that it looks like it raises from xarray, it's -# very acceptable to ignore it here. +# While we only raise an error on warnings from within xarray, if dependency +# raises a warning with a stacklevel such that it's interpreted to be raised +# from xarray, please feel free to ignore it here. # # If this gets in the way of making progress, it's also acceptable to # temporarily add additional ignores. From dd2548e2db5188fcd187744e406e7b831b19a7c2 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 28 Apr 2024 16:09:30 -0700 Subject: [PATCH 7/8] --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 53834342888..ccc9333c886 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -301,6 +301,7 @@ addopts = ["--strict-config", "--strict-markers"] filterwarnings = [ "error:::xarray.*", + "default:No index created:UserWarning:xarray.core.dataset", "default::UserWarning:xarray.tests.test_coding_times", "default::UserWarning:xarray.tests.test_computation", "default::UserWarning:xarray.tests.test_dataset", From 72bfc9feef7a68a6ce2a9afe03161d3199c76b97 Mon Sep 17 00:00:00 2001 From: Maximilian Roos Date: Sun, 28 Apr 2024 16:35:01 -0700 Subject: [PATCH 8/8] --- pyproject.toml | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ccc9333c886..00348ba7fb4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -286,17 +286,19 @@ addopts = ["--strict-config", "--strict-markers"] # We want to forbid warnings from within xarray in our tests — instead we should # fix our own code, or mark the test itself as expecting a warning. So this: # - Converts any warning from xarray into an error -# - Ignores warnings which the test suite currently raises; since it wasn't -# - practical to fix them all before merging this config. We can remove these -# over time -- if you see a warning that we now handle, remove the exception. -# If you are up for a small contribution, find the warning and fix it! The -# warnings are still listed (`default`, not `ignore`). +# - Allows some warnings ("default") which the test suite currently raises, +# since it wasn't practical to fix them all before merging this config. The +# arnings are still listed in CI (since it uses `default`, not `ignore`). +# +# We can remove these rules allowing warnings; a valued contribution is removing +# a line, seeing what breaks, and then fixing the library code or tests so that +# it doesn't raise warnings. # # While we only raise an error on warnings from within xarray, if dependency # raises a warning with a stacklevel such that it's interpreted to be raised -# from xarray, please feel free to ignore it here. +# from xarray, please feel free to add a rule switching it to `default` here. # -# If this gets in the way of making progress, it's also acceptable to +# If these settings get in the way of making progress, it's also acceptable to # temporarily add additional ignores. filterwarnings = [