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

Experiment with community workarounds to duplicated warnings #5499

Open
Tracked by #5464
trexfeathers opened this issue Sep 15, 2023 · 3 comments · May be fixed by #5506
Open
Tracked by #5464

Experiment with community workarounds to duplicated warnings #5499

trexfeathers opened this issue Sep 15, 2023 · 3 comments · May be fixed by #5506

Comments

@trexfeathers
Copy link
Contributor

trexfeathers commented Sep 15, 2023

See #5463 for write-up

@stephenworsley
Copy link
Contributor

A solution is proposed by #5506, this implements one of two potential replacements for the warnings.warn function. In particular, this mimics the behaviour you would get by calling warnings.simplefilter("default") (see https://docs.python.org/3/library/warnings.html#the-warnings-filter). Similarly, one could modify this function slightly to mimic the behaviour of warnings.simplefilter("once"). This would make the behaviour of each of these warnings more consistent within a context where they behaved as expected, however it comes at the cost of reduced flexibility when you want to apply a different context like warnings.simplefilter("always"). The table below descibes how each proposed function would behave in each context. Note, "sometimes" here means that the warnings do not behave as expected when this bus is present python/cpython#73858.

Method Handles "once" Handles "default" Handles "always"
warnings.warn sometimes sometimes sometimes
Forced default sometimes always never
Forced once always never never

An alternate approach is to rewrite the code in iris to avoid the bug described here python/cpython#73858. In particular, this patch of code appears to be resposible for the bug appearing during loading:

with warnings.catch_warnings(record=True) as warning_records:
cube.cell_methods = parse_cell_methods(nc_att_cell_methods)
# Filter to get the warning we are interested in.
warning_records = [
record
for record in warning_records
if issubclass(record.category, UnknownCellMethodWarning)
]
if len(warning_records) > 0:
# Output an enhanced warning message.
warn_record = warning_records[0]
name = "{}".format(cf_var.cf_name)
msg = warn_record.message.args[0]
msg = msg.replace("variable", "variable {!r}".format(name))
warnings.warn(message=msg, category=UnknownCellMethodWarning)

It is not guaranteed that all duplications would be a result of code within iris, since some calls to other libraries like import numpy or dask.array.map_blocks also introduce this bug. It may therefore be worthwhile taking both approaches so that:

  • warnings within iris are not duplicated due to code outsied of iris.
  • warnings outside of iris are not duplicated due to code within iris.

Finally, it is worth noting that this may end up with us having code that needs to be reverted once this bug is fixed within python, so this ought to be noted by a TODO alongside the code.

@stephenworsley
Copy link
Contributor

See this gist for a more in depth demonstration of warning duplication https://gist.github.com/stephenworsley/7716fdba4cc634e73454462ad60b3f67

@stephenworsley
Copy link
Contributor

The work done in #5536 resolved some of the warning duplication problems, but there remains a significant set of cases which will still cause warning duplication. In particular, this is highlighted by the test data file at NetCDF/mercator/toa_brightness_temperature.nc. This file contains an AuxCoord which, on load, contains lazy points. When merging happens as part of the load process this AuxCoord is copied. This calls a dask function which has a call to warnings.catch_warnings() which causes warnings to repeat. Since the source of this problem lies outside of iris, we are unable to solve this in the way we did with #5536. It is also likely that there may exist other similar problems caused by external code.

It is therefore worthwhile progressing the solution proposed in #5506 which would work around such problematic uses of catch_warnings() without having to remove them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Status: In Review
Development

Successfully merging a pull request may close this issue.

3 participants