Skip to content
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

Categorise warnings #5498

Merged
merged 16 commits into from
Sep 22, 2023
Merged

Categorise warnings #5498

merged 16 commits into from
Sep 22, 2023

Conversation

trexfeathers
Copy link
Contributor

@trexfeathers trexfeathers commented Sep 14, 2023

🚀 Pull Request

Description

Closes #5472

To do

  • Actually categorise all the warnings!
  • Ensure we have test coverage for the new subcategories by editing existing tests
    • Is this appropriate to do in a second PR?
    • If we lose coverage because there are NO existing tests for a certain warning, should we be writing new tests for that warning or is that scope creep?

Consult Iris pull request check list

@trexfeathers
Copy link
Contributor Author

f4e6a35

I think I got all of them. Unfortunately the regex test I wrote doesn't cope with any nested brackets in a warn() call, so that will still fail. Any suggestions for how else to achieve the below test?

def test_categorised_warnings():
"""
To ensure that all UserWarnings raised by Iris are categorised, for ease of use.
No obvious category? use the parent:
:class:`iris.exceptions.IrisUserWarning`.
Need a unique combination of categories?
Use :func:`~iris.exceptions.warning_combo` .
"""
warn_regex = re.compile(r"(?s)warn\(.*?\)")
class ResultTuple(NamedTuple):
file_path: Path
line_number: int
match_string: str
warns_without_category: list[ResultTuple] = []
warns_with_user_warning: list[ResultTuple] = []
for file_path in Path(IRIS_DIR).rglob("*.py"):
file_text = file_path.read_text()
matched = warn_regex.search(file_text)
while matched:
start, end = matched.span()
result_tuple = ResultTuple(
file_path=file_path,
line_number=file_text[:start].count("\n") + 1,
match_string=file_text[start:end],
)
if not re.search(r"category=", result_tuple.match_string):
warns_without_category.append(result_tuple)
if re.search(r"\bUserWarning\b", result_tuple.match_string):
warns_with_user_warning.append(result_tuple)
matched = warn_regex.search(file_text, start + 1)
# All warnings raised by Iris must be raised with a category kwarg.
# (This avoids UserWarnings being raised by unwritten default behaviour).
assert warns_without_category == []
# No warnings raised by Iris can be the base UserWarning class.
assert warns_with_user_warning == []

@trexfeathers
Copy link
Contributor Author

Location Warnings
fileformats/netcdf/saver.py | ? IrisUserWarning,
_concatenate.py | Overlap on concatenate axis IrisUserWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
aux_factory.py | Disregarding bounds IrisIgnoringBoundsWarning,
config.py | Ignoring config IrisIgnoringWarning,
config.py | Invalid value / defaulting IrisDefaultingWarning,
coord_systems.py | inverse_flattening limited effect IrisUserWarning,
coord_systems.py | Discard false_easting/northing IrisDefaultingWarning,
coords.py | Guessing bounds IrisGuessBoundsWarning,
coords.py | Metadata not fully descriptive IrisVagueMetadataWarning,
coords.py | Disregarding bounds (Metadata not fully descriptive) IrisVagueMetadataWarning,
coords.py | Metadata not fully descriptive IrisVagueMetadataWarning,
cube.py | Collapsing without weighting IrisUserWarning,
cube.py | Disregarding bounds IrisIgnoringBoundsWarning,
iterate.py | Coordinate values differ (but matching definitions) IrisUserWarning,
pandas.py | Pandas structure is series IrisIgnoringWarning,
plot.py | Plotting module might not be supported IrisUnsupportedPlottingWarning,
plot.py | Plotting function might not be supported IrisUnsupportedPlottingWarning,
analysis/_regrid.py | Dropped coordinates prevent update IrisImpossibleUpdateWarning,
analysis/calculus.py | Circular mismatch IrisUserWarning,
analysis/cartography.py | Assuming spherical Earth from ellipsoid IrisDefaultingWarning,
analysis/cartography.py | Assuming spherical Earth from ellipsoid IrisDefaultingWarning,
analysis/cartography.py | Default spherical Earth radius IrisDefaultingWarning,
analysis/cartography.py | Clipping out of range latitude IrisDefaultingWarning,
analysis/cartography.py | Defaulting coordinate system IrisDefaultingWarning,
analysis/cartography.py | Discarding coordinates IrisIgnoringWarning,
analysis/geometry.py | Geometry exceeds dimension IrisGeometryExceedWarning,
analysis/geometry.py | Geometry exceeds dimension IrisGeometryExceedWarning,
analysis/geometry.py | Geometry exceeds dimension IrisGeometryExceedWarning,
analysis/geometry.py | Geometry exceeds dimension IrisGeometryExceedWarning,
analysis/maths.py | Disregarding bounds IrisIgnoringBoundsWarning,
experimental/regrid.py | Dropped coordinates prevent update IrisImpossibleUpdateWarning,
fileformats/_ff.py | Assuming standard c grid IrisDefaultingWarning, IrisLoadWarning
fileformats/_ff.py | Incorrect coordinates used IrisLoadWarning,
fileformats/_ff.py | bdy below zero IrisLoadWarning,
fileformats/_ff.py | Assuming P grid IrisDefaultingWarning, IrisLoadWarning
fileformats/_ff.py | Picking P position as cell type IrisDefaultingWarning, IrisLoadWarning
fileformats/_ff.py | Partially missing coord values IrisLoadWarning,
fileformats/_ff.py | Skipped field IrisLoadWarning,
tests/graphics/idiff.py | Ignoring unregistered test result IrisIgnoringWarning,
fileformats/name_loaders.py | Unknown units IrisLoadWarning,
fileformats/name_loaders.py | Unable to create cell method IrisLoadWarning,
fileformats/nimrod_load_rules.py | Unhandled units recorded IrisNimrodTranslationWarning,
fileformats/nimrod_load_rules.py | Incomplete coordinate reference system IrisNimrodTranslationWarning,
fileformats/nimrod_load_rules.py | Assuming vertical coord type IrisNimrodTranslationWarning,
fileformats/nimrod_load_rules.py | No default units, metdata may be incomplete IrisNimrodTranslationWarning,
fileformats/pp.py | Mask value match IrisMaskValueMatchWarning, IrisLoadWarning
fileformats/pp.py | Downcasting array precision IrisDefaultingWarning, IrisLoadWarning
fileformats/pp.py | Missing landmask decompression IrisLoadWarning,
fileformats/pp.py | Uninterpretable field / skipping rest of file IrisLoadWarning, IrisIgnoringWarning
fileformats/pp.py | Inconsistent fields / skipping rest of file IrisLoadWarning, IrisIgnoringWarning
fileformats/pp_save_rules.py | PP conditional warning IrisPpClimModifiedWarning,
fileformats/rules.py | Multiple reference cubes IrisUserWarning,
fileformats/rules.py | Ignoring invalid units (PP) IrisIgnoringWarning,
fileformats/rules.py | Unable to create factory IrisUserWarning,
fileformats/_nc_load_rules/actions.py | Ignoring unrecognised formula IrisCfLoadWarning, IrisIgnoringWarning
fileformats/_nc_load_rules/actions.py | Skipping some hybrid factories / hybrid coords sharing variable IrisLoadWarning, IrisIgnoringWarning
fileformats/_nc_load_rules/helpers.py | Cell methods may be incorrectly parsed IrisCfLoadWarning,
fileformats/_nc_load_rules/helpers.py | Cell methods not fully parsed IrisCfLoadWarning,
fileformats/_nc_load_rules/helpers.py | Skipping global attribute IrisLoadWarning, IrisIgnoringWarning
fileformats/_nc_load_rules/helpers.py | Rotated pole not fully specified IrisCfLoadWarning,
fileformats/_nc_load_rules/helpers.py | Ignoring invalid units IrisCfLoadWarning, IrisIgnoringWarning
fileformats/_nc_load_rules/helpers.py | Ignoring climatology IrisCfLoadWarning, IrisIgnoringWarning
fileformats/_nc_load_rules/helpers.py | Filling masked points IrisDefaultingWarning, IrisLoadWarning
fileformats/_nc_load_rules/helpers.py | Filling masked points IrisDefaultingWarning, IrisLoadWarning
fileformats/_nc_load_rules/helpers.py | Demoting to aux coord IrisCfLoadWarning, IrisDefaultingWarning
fileformats/_nc_load_rules/helpers.py | Skipped coord - could not add IrisCannotAddWarning,
fileformats/_nc_load_rules/helpers.py | Skipped coord - could not add IrisCannotAddWarning,
fileformats/_nc_load_rules/helpers.py | Skipped coord - could not add IrisCannotAddWarning,
fileformats/_nc_load_rules/helpers.py | Skipped cell measure - could not add IrisCannotAddWarning,
fileformats/_nc_load_rules/helpers.py | Skipped ancil var - could not add IrisCannotAddWarning,
fileformats/_nc_load_rules/helpers.py | Invalid coord parameter IrisCfInvalidCoordParamWarning,
fileformats/_nc_load_rules/helpers.py | Invalid coord parameter IrisCfInvalidCoordParamWarning,
fileformats/_nc_load_rules/helpers.py | Invalid coord parameter IrisCfInvalidCoordParamWarning,
fileformats/_nc_load_rules/helpers.py | Invalid coord parameter IrisCfInvalidCoordParamWarning,
fileformats/netcdf/loader.py | Coordinate not found IrisFactoryCoordNotFoundWarning,
fileformats/netcdf/loader.py | Disregarding bounds IrisIgnoringBoundsWarning, IrisLoadWarning
fileformats/netcdf/loader.py | AuxFactory loading problem IrisLoadWarning,
fileformats/netcdf/saver.py | Mask value match IrisMaskValueMatchWarning,
fileformats/netcdf/saver.py | No cf patch defined IrisCfSaveWarning,
fileformats/netcdf/saver.py | Can't determine formula terms IrisSaveWarning,
fileformats/netcdf/saver.py | Unsupported coord system IrisSaveWarning,
fileformats/netcdf/saver.py | Unsupported coord system IrisSaveWarning,
fileformats/netcdf/saver.py | Must be global attribute IrisCfSaveWarning,
fileformats/netcdf/saver.py | No cf patch conventions defined IrisCfSaveWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Missing cf variable IrisCfMissingVarWarning,
fileformats/cf.py | Recommend NetCDF4 IrisLoadWarning,
fileformats/cf.py | Non-spanning dimensions / ignoring variable IrisCfNonSpanningVarWarning,
fileformats/cf.py | Non-spanning dimensions / ignoring variable IrisCfNonSpanningVarWarning,
experimental/ugrid/cf.py | Missing cf variable IrisCfMissingVarWarning,
experimental/ugrid/cf.py | Ignoring cf variable IrisCfLabelVarWarning,
experimental/ugrid/cf.py | Missing cf variable IrisCfMissingVarWarning,
experimental/ugrid/cf.py | Ignoring cf variable IrisCfLabelVarWarning,
experimental/ugrid/cf.py | Missing cf variable IrisCfMissingVarWarning,
experimental/ugrid/cf.py | Ignoring cf variable IrisCfLabelVarWarning,
experimental/ugrid/load.py | Incorrect cf_role, defaulting IrisCfWarning, IrisDefaultingWarning
experimental/ugrid/load.py | No topology_dimension, assuming IrisCfWarning, IrisDefaultingWarning
experimental/ugrid/load.py | Wrong topology_dimension, replacing IrisCfWarning, IrisDefaultingWarning, IrisIgnoringWarning

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Patch coverage: 83.42% and project coverage change: +0.04% 🎉

Comparison is base (10cec69) 89.37% compared to head (01db6a9) 89.41%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5498      +/-   ##
==========================================
+ Coverage   89.37%   89.41%   +0.04%     
==========================================
  Files          89       89              
  Lines       22446    22539      +93     
  Branches     5387     5387              
==========================================
+ Hits        20061    20154      +93     
  Misses       1639     1639              
  Partials      746      746              
Files Changed Coverage Δ
lib/iris/__init__.py 88.88% <ø> (ø)
lib/iris/analysis/geometry.py 79.04% <ø> (ø)
lib/iris/analysis/maths.py 89.73% <ø> (ø)
lib/iris/aux_factory.py 77.82% <26.66%> (+0.02%) ⬆️
lib/iris/analysis/cartography.py 86.41% <33.33%> (ø)
lib/iris/plot.py 86.45% <33.33%> (ø)
lib/iris/analysis/_regrid.py 83.37% <50.00%> (+0.03%) ⬆️
lib/iris/analysis/calculus.py 85.86% <50.00%> (+0.05%) ⬆️
lib/iris/experimental/regrid.py 77.51% <50.00%> (+0.13%) ⬆️
lib/iris/fileformats/netcdf/saver.py 89.14% <60.00%> (+0.02%) ⬆️
... and 21 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@trexfeathers
Copy link
Contributor Author

fb63268

I have confirmed using text searches that the fixed test is finding all ... warn( ... calls

@trexfeathers
Copy link
Contributor Author

37df50b

There are still many tests asserting for UserWarnings. CI passes at this commit demonstrate that the sub-categorisation is a non-breaking change - any code expecting a UserWarning is still getting one.

@trexfeathers
Copy link
Contributor Author

Coverage

I have not made a concerted effort to have tests for all the new warning sub-categories. I did have to reference several of them in 37df50b just to get the tests to work.

Codecov seems happy with test coverage (#5498 (comment)) already. As noted, there are still many tests asserting for UserWarnings that I could change. I don't believe we have tests for all ~130 of our different warnings, however the existing tests may or may-not cover all the new sub-categories if I made them more specialised.

Options

@HGWright?

  1. The coverage report already looks good, no further action.
  2. Replace all assertions for UserWarnings with more specific sub-categories.
  3. Do 2, AND write tests for any sub-categories not already covered.

@trexfeathers trexfeathers marked this pull request as ready for review September 20, 2023 14:53
Copy link
Contributor

@HGWright HGWright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @trexfeathers, this is going to add lots of flexibility for filtering warnings.

@HGWright HGWright merged commit 4f126a0 into SciTools:main Sep 22, 2023
17 checks passed
@trexfeathers
Copy link
Contributor Author

Thanks for wading through, @HGWright!

tkknight added a commit to tkknight/iris that referenced this pull request Oct 26, 2023
* upstream/main:
  moved latest warning banner logic to conf.py (SciTools#5508)
  updated layout of top navbar (SciTools#5505)
  Oblique and Rotated Mercator (SciTools#5548)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5549)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5527)
  Bump scitools/workflows from 2023.09.1 to 2023.10.0 (SciTools#5540)
  nep29 drop table schedule numpy>1.21 (SciTools#5525)
  Updated environment lockfiles (SciTools#5545)
  Replaced `NotImplementedError` with `NotImplemented` (SciTools#5544)
  Gallery: show colour bar stealing space from multiple axes (SciTools#5537)
  Updated environment lockfiles (SciTools#5524)
  Set some memory benchmarks to on-demand to reduce noise. (SciTools#5481)
  updating docs and stale comment (SciTools#5522)
  Ensure removal of release candidate from What's New title. (SciTools#5526)
  Updated environment lockfiles (SciTools#5513)
  Docs page on filtering warnings (SciTools#5509)
  Replaced pkg_resources version parser with packager version parser. (SciTools#5511)
  Categorise warnings (SciTools#5498)
  Updated all np.product calls to np.prod (SciTools#5493)
@trexfeathers trexfeathers deleted the categorise_warnings branch May 3, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Granular categorisation of Iris warnings
2 participants