Skip to content

[WIP] Silence userwarnings#3139

Closed
tv3141 wants to merge 9 commits intoSciTools:masterfrom
tv3141:silence_userwarnings
Closed

[WIP] Silence userwarnings#3139
tv3141 wants to merge 9 commits intoSciTools:masterfrom
tv3141:silence_userwarnings

Conversation

@tv3141
Copy link
Contributor

@tv3141 tv3141 commented Aug 14, 2018

Closes #3078

  • create IrisUserWarning and uses it where a UserWarning was raised before. This allows the silencing of all IrisUserWarnings from Iris without silencing UserWarnings from other libraries.
  • silence all IrisUserWarnings (this can be made more specific)

@tv3141 tv3141 force-pushed the silence_userwarnings branch from 4a47fe1 to f1f1fb9 Compare September 20, 2018 11:28
@DPeterK
Copy link
Member

DPeterK commented Oct 3, 2018

Hi @tv3141 - thanks for this work to fix an outstanding Iris issue! Looks like some changes to master mean that your PR now has some merge conflicts; would you mind rebasing to fix these conflicts? Thanks!

@corinnebosley
Copy link
Member

@tv3141 This PR still looks good to me, but we need you to rebase please. This isn't as straightforward as the last two that I've rebased for you; I'll need you to make sure the conflicts are resolved correctly.

@tv3141
Copy link
Contributor Author

tv3141 commented Oct 15, 2018

@corinnebosley @dkillick I will rebase this as soon as I can.

@tv3141 tv3141 force-pushed the silence_userwarnings branch from f1f1fb9 to 8285298 Compare October 17, 2018 21:08
@tv3141 tv3141 force-pushed the silence_userwarnings branch from 8285298 to 2811605 Compare October 17, 2018 21:50
@tv3141
Copy link
Contributor Author

tv3141 commented Oct 17, 2018

This still needs a bit of work.

Also some tests fail because of ncdump. I do not know how my changes cause that yet.

ERROR: test_conflicting_global_attributes (iris.tests.test_netcdf.TestNetCDFSave)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/test-environment/lib/python3.6/site-packages/scitools_iris-2.3.0.dev0-py3.6.egg/iris/tests/test_netcdf.py", line 779, in test_conflicting_global_attributes
    ('netcdf', 'netcdf_save_confl_global_attr.cdl'))
  File "/home/travis/miniconda/envs/test-environment/lib/python3.6/site-packages/scitools_iris-2.3.0.dev0-py3.6.egg/iris/tests/__init__.py", line 355, in assertCDL
    stderr=cdl_file, stdout=cdl_file)
  File "/home/travis/miniconda/envs/test-environment/lib/python3.6/subprocess.py", line 291, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['ncdump', '-h', '/tmp/tmpaa88ge1z.nc']' returned non-zero exit status 1.

@corinnebosley
Copy link
Member

Yeah that's a bit worrying isn't it? I'll look into the testing environment; could be that something's been updated which has broken subprocess.

@corinnebosley
Copy link
Member

On second thoughts I'll leave it a bit and have another look if and when that's the only test which fails.

> find lib -type f -exec sed -i 's/UserWarning/IrisUserWarning/g' {} +
> find lib -type f -exec sed -i 's/warn(msg)/warn(msg, IrisUserWarning)/g' {} +
1) Find warnings not using IrisUserWarning
> ack --python 'warn\((?!(.|\n|{})*(Warning|IrisDeprecation))' -A 1

2) Reformat to:
```python
msg = '...'
warnings.warn(msg, IrisUserWarning, ...)
```
@tv3141 tv3141 force-pushed the silence_userwarnings branch from a37a22d to 45d6bd3 Compare November 7, 2018 17:19
@tv3141
Copy link
Contributor Author

tv3141 commented Nov 7, 2018

Yeah that's a bit worrying isn't it? I'll look into the testing environment; could be that something's been updated which has broken subprocess.

The problem was that the file did not exists. The root cause was missing indentation. I added #3219 to make this more verbose.

Copy link
Contributor Author

@tv3141 tv3141 left a comment

Choose a reason for hiding this comment

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

So far I followed the warning docs on testing. This results in brittle tests that fail when the executed code creates FutureWarnings like we see them currently.
I think it is better to look for the warnings I want to test and re-emit all other warnings so that they are shown in the test log: 9eb61dd

@bjlittle
Copy link
Member

@tv3141 Firstly, thanks for this awesome PR 👍

Sadly it's went rather stale. Also, we've decided to adopt logging as an alternative strategy, and reserve warnings for limited use throughout the code base. As such I'm going to close this PR, but reference it's efforts in an associated issue.

Thanks again

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.

silence intended UserWarnings in tests

5 participants