Categorise warnings by trexfeathers · Pull Request #5498 · SciTools/iris

trexfeathers · 2023-09-14T09:21:25Z

🚀 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 · 2023-09-14T14:56:31Z

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?

iris/lib/iris/tests/test_coding_standards.py

Lines 137 to 181 in f4e6a35

    
           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 · 2023-09-15T15:45:01Z

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 · 2023-09-20T12:39:27Z

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 · 2023-09-20T12:42:24Z

`fb63268`

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

trexfeathers · 2023-09-20T12:44:51Z

`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 · 2023-09-20T12:51:06Z

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?

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

HGWright

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

trexfeathers · 2023-09-22T10:32:37Z

Thanks for wading through, @HGWright!

* 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 added 5 commits September 11, 2023 12:37

Introduce IrisUserWarning.

75fe49c

Align existing warning subcategories.

6bc3dff

Plant a flag for future DeprecationWarning use.

1c1afcb

Introduce test_categorised_warnings().

131bf7d

Merge remote-tracking branch 'upstream/main' into categorise_warnings

31a6491

trexfeathers requested a review from HGWright September 14, 2023 09:21

trexfeathers added 2 commits September 14, 2023 13:27

Fix backwards compatibility of existing warnings classes.

54ebf1f

Categorise all Iris warnings.

f4e6a35

trexfeathers added 6 commits September 19, 2023 16:17

Fix test_categorised_warnings() by using ast module.

fb63268

Add missing warning category kwargs.

d5a67e0

Warnings combo experiment.

8f0756c

Warnings combo finalise.

8a844f1

Fix stray comma in coords.py.

7408814

Fix failing tests.

37df50b

Categorise warning tests.

257be15

trexfeathers mentioned this pull request Sep 20, 2023

Small warnings refactor at next major release #5507

Open

trexfeathers and others added 2 commits September 20, 2023 15:42

Merge branch 'main' into categorise_warnings

c00ae51

What's New entry.

01db6a9

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

trexfeathers mentioned this pull request Sep 21, 2023

Docs page on filtering warnings #5509

Merged

HGWright approved these changes Sep 22, 2023

View reviewed changes

HGWright merged commit 4f126a0 into SciTools:main Sep 22, 2023

trexfeathers deleted the categorise_warnings branch May 3, 2024 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Categorise warnings#5498

Categorise warnings#5498
HGWright merged 16 commits intoSciTools:mainfrom
trexfeathers:categorise_warnings

trexfeathers commented Sep 14, 2023 •

edited

Loading

Uh oh!

trexfeathers commented Sep 14, 2023

Uh oh!

trexfeathers commented Sep 15, 2023

Uh oh!

codecov bot commented Sep 20, 2023 •

edited

Loading

Uh oh!

trexfeathers commented Sep 20, 2023

Uh oh!

trexfeathers commented Sep 20, 2023

Uh oh!

trexfeathers commented Sep 20, 2023

Uh oh!

HGWright left a comment

Uh oh!

trexfeathers commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

trexfeathers commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Pull Request

Description

To do

Uh oh!

trexfeathers commented Sep 14, 2023

Uh oh!

trexfeathers commented Sep 15, 2023

Uh oh!

codecov bot commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

trexfeathers commented Sep 20, 2023

fb63268

Uh oh!

trexfeathers commented Sep 20, 2023

37df50b

Uh oh!

trexfeathers commented Sep 20, 2023

Coverage

Options

Uh oh!

HGWright left a comment

Choose a reason for hiding this comment

Uh oh!

trexfeathers commented Sep 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

trexfeathers commented Sep 14, 2023 •

edited

Loading

codecov bot commented Sep 20, 2023 •

edited

Loading

`fb63268`

`37df50b`