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

Tests: Improved tests by converting warnings to errors #4113

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

t20100
Copy link
Member

@t20100 t20100 commented Apr 19, 2024

Checklist:


This PR aim at running tests with stricter rules by raising errors for warnings (e.g. deprecation warnings).
This is done by configuring pytest, adding ignores on specific tests and silenting warnings in silx.utils.number

Thanks @woutdenolf for the idea!

@t20100 t20100 added this to the Next release milestone Apr 19, 2024
Comment on lines +109 to +119
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "overflow encountered in scalar power", RuntimeWarning)
precision = _biggest_float(10) ** nb_precision_digits * 1.2
previous_type = _biggest_float
for numpy_type in _float_types:
if numpy_type == _biggest_float:
# value was already casted using the bigger type
continue
reduced_value = numpy_type(value)
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "overflow encountered in cast", RuntimeWarning)
reduced_value = numpy_type(value)
Copy link
Member Author

Choose a reason for hiding this comment

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

Having tests for raise those warnings, I expects that overflows were taken into accounts when writing the code 🤷

Copy link
Contributor

Choose a reason for hiding this comment

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

The docstring says "It has some known issues: precision loss." so I guess.

@@ -1530,6 +1530,7 @@ def testBoundingRectArguments(self):
with self.assertRaises(Exception):
item.setBounds((-1000, 1000, 2000, -2000))

@pytest.mark.filterwarnings("ignore:Attempting to set identical low and high ylims makes transformation singular; automatically expanding.:UserWarning")
Copy link
Member Author

Choose a reason for hiding this comment

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

Ignore warnings only during tests: Those 3 warnings are about erroneous conditions so thet are not ignored in silx code.

Comment on lines +26 to +27
'ignore:tostring\(\) is deprecated. Use tobytes\(\) instead\.:DeprecationWarning:OpenGL',
'ignore:Jupyter is migrating its paths to use standard platformdirs:DeprecationWarning',
Copy link
Member Author

Choose a reason for hiding this comment

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

Listing here general warnings from our dependencies

Copy link
Contributor

Choose a reason for hiding this comment

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

For the jupyter path warnings you do not specify the module but I guess there are several? I don't remember but in our CI I do this instead of ignoring the warnings:

test-3.11:
  extends: .test-3.11_glx
  variables:
    JUPYTER_PLATFORM_DIRS: 1

I'm fine with it either way.

Copy link
Member Author

Choose a reason for hiding this comment

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

AFAIR, it occurs once through an import, during the test collections I think.
But indeed, the env var does the job

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll keep it like that for now: it's set at a single place instead of appveyor+github configs, and also works when running locally.

@t20100 t20100 marked this pull request as ready for review April 19, 2024 14:26
@t20100 t20100 requested a review from woutdenolf April 19, 2024 14:26
Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

That wasn't too bad. Nice!

Comment on lines +26 to +27
'ignore:tostring\(\) is deprecated. Use tobytes\(\) instead\.:DeprecationWarning:OpenGL',
'ignore:Jupyter is migrating its paths to use standard platformdirs:DeprecationWarning',
Copy link
Contributor

Choose a reason for hiding this comment

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

For the jupyter path warnings you do not specify the module but I guess there are several? I don't remember but in our CI I do this instead of ignoring the warnings:

test-3.11:
  extends: .test-3.11_glx
  variables:
    JUPYTER_PLATFORM_DIRS: 1

I'm fine with it either way.

Comment on lines +109 to +119
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "overflow encountered in scalar power", RuntimeWarning)
precision = _biggest_float(10) ** nb_precision_digits * 1.2
previous_type = _biggest_float
for numpy_type in _float_types:
if numpy_type == _biggest_float:
# value was already casted using the bigger type
continue
reduced_value = numpy_type(value)
with warnings.catch_warnings():
warnings.filterwarnings("ignore", "overflow encountered in cast", RuntimeWarning)
reduced_value = numpy_type(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

The docstring says "It has some known issues: precision loss." so I guess.

@t20100 t20100 merged commit f97ae17 into silx-kit:main Apr 30, 2024
7 of 8 checks passed
@t20100 t20100 deleted the pytest-warnings branch April 30, 2024 15:26
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.

2 participants