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

Mitigate impact of #342 #343

Merged
merged 8 commits into from
Sep 20, 2022
Merged

Mitigate impact of #342 #343

merged 8 commits into from
Sep 20, 2022

Conversation

bwohlberg
Copy link
Collaborator

Mitigate impact of #342.

@bwohlberg bwohlberg added bug Something isn't working tests Pertaining to SCICO tests labels Sep 19, 2022
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #343 (57b23c2) into main (e649b42) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #343      +/-   ##
==========================================
+ Coverage   93.95%   93.96%   +0.01%     
==========================================
  Files          58       58              
  Lines        3493     3499       +6     
==========================================
+ Hits         3282     3288       +6     
  Misses        211      211              
Flag Coverage Δ
unittests 93.96% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
scico/denoiser.py 89.77% <100.00%> (+0.74%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@@ -5,7 +5,26 @@
# user license can be found in the 'LICENSE' file distributed with the
# package.

"""Denoisers."""
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

The user sees this when looking carefully at the source code or with help(scico.denoiser). Do you think this is enough? Perhaps if these packages are installed there could be an additional warning at the time of import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The user should also see this when looking at the online API docs for scico.denoisers. Good question as to whether it's enough. A warning on importing the module is not a bad idea.

sufficient to inflict this loss of numerical precision on all other
calculations run within the same process, even if :func:`bm4d` is not
actually used. Users who are not making use of :func:`bm4d` are
advised not to install the corresponding package. For additional information, see `scico issue #342 <https://github.com/lanl/scico/issues/342>`__.
Copy link
Contributor

Choose a reason for hiding this comment

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

line too long

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

)
warnings.filterwarnings(
action="ignore",
message="^The value of the smallest subnormal",
Copy link
Contributor

Choose a reason for hiding this comment

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

is this caret character there on purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, message is a regex.

@bwohlberg bwohlberg merged commit 391f0e5 into main Sep 20, 2022
@bwohlberg bwohlberg deleted the brendt/bm3d branch September 20, 2022 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tests Pertaining to SCICO tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants