search

LumiSpy / lumispy

Luminescence data analysis with HyperSpy.
https://lumispy.org
GNU General Public License v3.0
26 stars 17 forks source link

Fixing deprecation warning #114

Closed jordiferrero closed 2 years ago

jordiferrero commented 2 years ago

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

codecov-commenter commented 2 years ago

Codecov Report

Merging #114 (53ebdc7) into main (699be7d) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #114   +/-   ##
=======================================
  Coverage   95.70%   95.70%           
=======================================
  Files          11       11           
  Lines         582      582           
=======================================
  Hits          557      557           
  Misses         25       25
Impacted Files Coverage Δ
lumispy/signals/luminescence_spectrum.py 90.05% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 699be7d...53ebdc7. Read the comment docs.

jordiferrero commented 2 years ago

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?

jlaehne commented 2 years ago

It looks to me, that this method is not very clear; it would benefit from rewriting and possibly changing its behaviour:

Indeed, creating a variable background that contains the x and y vectors and afterwards splitting it again is not very intuitive (and avoiding that would along the way solve the problem with the deprecation warning.

I agree with @ericpre that the "HyperSpy" way should be to provide a method where a Signal1D object is subtracted from another Signal1D object. That would make it much more valuable for others to reuse. If the axes don't match (in size, offset and scale for UniformDataAxis), the background signal should be rebinned/interpolated to the main signal axis. This would also ease an extension to work in non-uniform data axes in the future.

I would propose in fact writing a new method remove_background_signal to do the job, and adding a deprecation warning to this method in order not "surprise" anyone with the change.

ericpre commented 2 years ago

I would propose in fact writing a new method remove_background_signal to do the job, and adding a deprecation warning to this method in order not "surprise" anyone with the change.

Other alternative would be:

jlaehne commented 2 years ago

As this was originally intended as a quick fix for some warnings, I would propose the following way forward

ericpre commented 2 years ago

@jordiferrero, do you think, this is feasible to finish this PR for the 0.2 release, i. e. in the coming days?

jordiferrero commented 2 years ago

I'll work on it tomorrow @ericpre Sorry I've been out of coding for a while recently :-(

jordiferrero commented 2 years ago

@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 :-)

jlaehne commented 2 years ago

@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 :-)

  1. It is an issue with the black formatting. In this case probably too long lines and easy to fix. I usually run black xy.py on the failing files to fix it automatically.
jlaehne commented 2 years ago

You can also try to set up an automatic black formatting on your system. See also: https://hyperspy.org/hyperspy-doc/current/dev_guide/coding_style.html https://kikuchipy.org/en/stable/contributing.html#code-style