-
Notifications
You must be signed in to change notification settings - Fork 18
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
Fixing deprecation warning #114
Conversation
Tests failing
Codecov Report
@@ Coverage Diff @@
## main #114 +/- ##
=======================================
Coverage 95.70% 95.70%
=======================================
Files 11 11
Lines 582 582
=======================================
Hits 557 557
Misses 25 25
Continue to review full report at Codecov.
|
I believe the DepreciationWarning caused by luminescence_spectrum.py has been fixed.
which seems to come from numpy, not us. Is that correct? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at CI, the warning is still there. This PR slightly improves the syntax and make this function more readable.
It looks to me, that this method is not very clear; it would benefit from rewriting and possibly changing its behaviour:
- the name
remove_background_from_file
implies that we would provide a file, but the docstring only mention arrays or Signal1D for thebackground
argument - the flexibility in supporting different shape for the
background
argument is confusing.
As alternative to the current behaviour, I would suggest to support only Signal1D
and add a separate method to read the background from a file, which returns a Signal1D. This would have the advantage of being transparent/explicit for the users.
I believe the DepreciationWarning caused by luminescence_spectrum.py has been fixed. There is still the following warning:
lumispy/tests/signals/test_luminescence_spectrum.py::TestLumiSpectrum::test_errors_raise /opt/hostedtoolcache/Python/3.9.10/x64/lib/python3.9/site-packages/numpy/core/fromnumeric.py:1970: VisibleDeprecationWarning: Creating an ndarray from ragged nested sequences (which is a list-or-tuple of lists-or-tuples-or ndarrays with different lengths or shapes) is deprecated. If you meant to do this, you must specify 'dtype=object' when creating the ndarray. result = asarray(a).shape
which seems to come from numpy, not us. Is that correct?
It most likely means that we are not using it as it is now expected to be used and there is some changes required in lumispy to remove this warning.
The description in #114 (comment) doesn't match with what this PR does! ;)
Indeed, creating a variable I agree with @ericpre that the "HyperSpy" way should be to provide a method where a I would propose in fact writing a new method |
Other alternative would be:
|
As this was originally intended as a quick fix for some warnings, I would propose the following way forward
|
@jordiferrero, do you think, this is feasible to finish this PR for the 0.2 release, i. e. in the coming days? |
I'll work on it tomorrow @ericpre |
@jlaehne Not it is ready for review. Can you maybe help me tell how to fix the "linting" error? That's something new to me :-) |
|
You can also try to set up an automatic black formatting on your system. See also: |
Description of the change
Try to fix depreciation warning #54 .
Depreciate the
remove_background_from_file
method. It will be replaced by a new better function using the HyperSpy syntaxis.Progress of the PR