Fix bug in _build_spectrum #66

LMSC-NTappy · 2022-11-17T08:58:58Z

Description of the change

Fixes the same bug as hyperspy/hyperspy#3060

Progress of the PR

Change implemented (can be split into several points),
[x add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
Check formatting changelog entry in the readthedocs doc build of this PR (link in github checks)
add tests,
ready for review.

Minimal example of the bug fix or the new feature

See hyperspy/hyperspy#3060

codecov · 2022-11-17T09:01:41Z

Codecov Report

Base: 84.88% // Head: 84.76% // Decreases project coverage by -0.13% ⚠️

Coverage data is based on head (a8fdee7) compared to base (680843b).
Patch coverage: 65.78% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #66      +/-   ##
==========================================
- Coverage   84.88%   84.76%   -0.13%     
==========================================
  Files          73       73              
  Lines        8892     8898       +6     
  Branches     1907     1955      +48     
==========================================
- Hits         7548     7542       -6     
- Misses        877      893      +16     
+ Partials      467      463       -4

Impacted Files	Coverage Δ
rsciio/digitalsurf/_api.py	`66.57% <65.78%> (-4.86%)`	⬇️
rsciio/tvips/_api.py	`98.48% <0.00%> (+<0.01%)`	⬆️
rsciio/bruker/_api.py	`88.29% <0.00%> (+0.31%)`	⬆️
rsciio/trivista/_api.py	`99.19% <0.00%> (+0.53%)`	⬆️
rsciio/usid/_api.py	`92.04% <0.00%> (+1.18%)`	⬆️

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

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

jlaehne · 2022-11-17T09:49:41Z

Would it make sense to test for the correct handling of the indexing?

Note that the .sur format is one of those which are not so well covered by tests, see #60
Would it possible for you (in a separate PR) to add more tests for uncovered parts? See the following link for the coverage report of this plugin: https://app.codecov.io/gh/hyperspy/rosettasciio/blob/main/rsciio/sur/_api.py

PS: Lint is unhappy with your new formatting ;-)

ericpre · 2023-02-11T13:11:23Z

I rebased to solve the merge conflict. There is an error in the test suite:

______________________ test_load_spectral_map_compressed _______________________
[gw1] linux -- Python 3.8.16 /opt/hostedtoolcache/Python/3.8.16/x64/bin/python

    def test_load_spectral_map_compressed():
        fname = os.path.join(
            MY_PATH, "digitalsurf_data", "test_spectral_map_compressed.sur"
        )
>       s = hs.load(fname)

rsciio/tests/test_digitalsurface.py:235: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:[51](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:52)7: in load
    objects = [load_single_file(filename, lazy=lazy, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:517: in <listcomp>
    objects = [load_single_file(filename, lazy=lazy, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:[57](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:58)6: in load_single_file
    return load_with_reader(filename=filename, reader=reader, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:[59](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:60)7: in load_with_reader
    file_data_list = importlib.import_module(reader["api"]).file_reader(filename,
rsciio/digitalsurf/_api.py:1267: in file_reader
    surdict = ds._build_sur_dict()
rsciio/digitalsurf/_api.py:512: in _build_sur_dict
    self._build_hyperspectral_map()
rsciio/digitalsurf/_api.py:636: in _build_hyperspectral_map
    self.signal_dict["original_metadata"] = self._build_original_metadata()
rsciio/digitalsurf/_api.py:928: in _build_original_metadata
    parsedict = self._MS_parse(a["_[60](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:61)_Comment"], "$", "=")
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

strMS = '@@SPECTRUMNAMES§@@\n\n$_WAFER\n$Lot Number = LOT_0000\n$ID = WAFER_0000\n$Type = ROUNDWITHFLAT\n$Center Position X = ...e Calibration Range_Y = 9.200000 V\n$Start_X = 0\n$Size_X = 1024\n$Start_Y = 0\n$Size_Y = 1024\n$Rotate = 0.000000 deg'
prefix = '$', delimiter = '='

    @staticmethod
    def _MS_parse(strMS, prefix, delimiter):
        """Parses a string containing metadata information. The string can be
        read from the comment section of a .sur file, or, alternatively, a file
        containing them with a similar formatting.
    
        Parameters
        ----------
        strMS: string containing metadata
        prefix: string (or char) character assumed to start each line.
        '$' if a .sur file.
        delimiter: string that delimits the keyword from value. always '='
    
        Returns
        -------
        dict_ms: dictionnary in the correct hyperspy metadata format
    
        """
        # dict_ms is created as an empty dictionnary
        dict_ms = {}
        # Title lines start with an underscore
        TITLESTART = "{:s}_".format(prefix)
    
        for line in strMS.splitlines():
            # Here we ignore any empty line or line starting with @@
            ignore = False
            if not line.strip() or line.startswith("@@"):
                ignore = True
            # If the line must not be ignored
            if not ignore:
                if line.startswith(TITLESTART):
                    # We strip keys from whitespace at the end and beginning
                    dict_ms = line[len(TITLESTART) :].strip()
>                   dict_ms[dict_ms] = {}
E                   TypeError: 'str' object does not support item assignment

rsciio/digitalsurf/_api.py:1031: TypeError
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
____________________________ test_load_spectral_map ____________________________
[gw1] linux -- Python 3.8.16 /opt/hostedtoolcache/Python/3.8.16/x64/bin/python

    def test_load_spectral_map():
        fname = os.path.join(MY_PATH, "digitalsurf_data", "test_spectral_map.sur")
>       s = hs.load(fname)

rsciio/tests/test_digitalsurface.py:283: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:517: in load
    objects = [load_single_file(filename, lazy=lazy, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:517: in <listcomp>
    objects = [load_single_file(filename, lazy=lazy, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:576: in load_single_file
    return load_with_reader(filename=filename, reader=reader, **kwds)
/opt/hostedtoolcache/Python/3.8.16/x64/lib/python3.8/site-packages/hyperspy/io.py:597: in load_with_reader
    file_data_list = importlib.import_module(reader["api"]).file_reader(filename,
rsciio/digitalsurf/_api.py:1267: in file_reader
    surdict = ds._build_sur_dict()
rsciio/digitalsurf/_api.py:512: in _build_sur_dict
    self._build_hyperspectral_map()
rsciio/digitalsurf/_api.py:[63](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:64)6: in _build_hyperspectral_map
    self.signal_dict["original_metadata"] = self._build_original_metadata()
rsciio/digitalsurf/_api.py:928: in _build_original_metadata
    parsedict = self._MS_parse(a["_60_Comment"], "$", "=")
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

strMS = '@@SPECTRUMNAMES§@@\n\n$_WAFER\n$Lot Number = LOT_0000\n$ID = WAFER_0000\n$Type = ROUNDWITHFLAT\n$Center Position X = ...e Calibration Range_Y = 9.200000 V\n$Start_X = 0\n$Size_X = 1024\n$Start_Y = 0\n$Size_Y = 1024\n$Rotate = 0.000000 deg'
prefix = '$', delimiter = '='

    @staticmethod
    def _MS_parse(strMS, prefix, delimiter):
        """Parses a string containing metadata information. The string can be
        read from the comment section of a .sur file, or, alternatively, a file
        containing them with a similar formatting.
    
        Parameters
        ----------
        strMS: string containing metadata
        prefix: string (or char) character assumed to start each line.
        '$' if a .sur file.
        delimiter: string that delimits the keyword from value. always '='
    
        Returns
        -------
        dict_ms: dictionnary in the correct hyperspy metadata format
    
        """
        # dict_ms is created as an empty dictionnary
        dict_ms = {}
        # Title lines start with an underscore
        TITLESTART = "{:s}_".format(prefix)
    
        for line in strMS.splitlines():
            # Here we ignore any empty line or line starting with @@
            ignore = False
            if not line.strip() or line.startswith("@@"):
                ignore = True
            # If the line must not be ignored
            if not ignore:
                if line.startswith(TITLESTART):
                    # We strip keys from whitespace at the end and beginning
                    dict_ms = line[len(TITLESTART) :].strip()
>                   dict_ms[dict_ms] = {}
E                   TypeError: 'str' object does not support item assignment

rsciio/digitalsurf/_api.py:1031: TypeError
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:5[79](https://github.com/hyperspy/rosettasciio/actions/runs/4151444898/jobs/7181754420#step:10:80) If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
------------------------------ Captured log call -------------------------------
ERROR    hyperspy.io:io.py:579 If this file format is supported, please report this error to the HyperSpy developers.
=============================== warnings summary ===============================

LMSC-NTappy · 2023-02-13T14:15:57Z

Thanks for letting me know

jlaehne · 2023-04-25T11:49:50Z

any progress on this one?

Attolight-NTappy

Woops, just checked and the bug fixing change has slipped into #98. PR can be closed.

I suggest that I update the changelog of #98 to signal that this bug is fixed to deal with this easily.

Sorry for the mess, I was inbetween position and github accounts change so the VCS management went a bit messy on my side...

Attolight-NTappy · 2023-04-25T15:35:29Z

rsciio/digitalsurf/_api.py

-        self.signal_dict["data"] = np.squeeze(
-            hypdic["_62_points"].reshape(
-                hypdic["_19_Number_of_Lines"],
-                hypdic["_18_Number_of_Points"],
-            )
-        )
+        data_shape = (hypdic["_19_Number_of_Lines"], hypdic["_18_Number_of_Points"])
+        data_array = np.squeeze(hypdic["_62_points"].reshape(data_shape, order="C"))
+
+        self.signal_dict["data"] = data_array


this is the only lines of code actually fixing code. They are already implemented in #98

Rest of the modifications (including one causing test suite error) are improvement on naming conventions that went wrong.

This PR can be closed

LMSC-NTappy mentioned this pull request Nov 17, 2022

Fix a bug in .sur files for 1D series of spectra hyperspy/hyperspy#3060

Merged

4 tasks

jlaehne approved these changes Nov 17, 2022

View reviewed changes

LMSC-NTappy and others added 4 commits February 11, 2023 12:54

Fix bug in _build_spectrum

227d9d8

Added 66.bugfix.rst to upcoming_changes

2dbc09e

Fix bug in _build_spectrum

895716b

Fix formatting using black

a8fdee7

ericpre force-pushed the main branch from 30246b8 to a8fdee7 Compare February 11, 2023 12:56

jlaehne added this to the v0.1.0 initial release milestone Apr 6, 2023

jlaehne added type: bug-fix status: waiting for author labels Apr 17, 2023

jlaehne mentioned this pull request Apr 17, 2023

Initial Release #99

Closed

19 tasks

Attolight-NTappy reviewed Apr 25, 2023

View reviewed changes

jlaehne closed this Apr 25, 2023

jlaehne removed status: waiting for author type: bug-fix labels Apr 25, 2023

jlaehne removed this from the v0.1.0 initial release milestone Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug in _build_spectrum #66

Fix bug in _build_spectrum #66

LMSC-NTappy commented Nov 17, 2022 •

edited

Loading

codecov bot commented Nov 17, 2022 •

edited

Loading

jlaehne commented Nov 17, 2022

ericpre commented Feb 11, 2023

LMSC-NTappy commented Feb 13, 2023

jlaehne commented Apr 25, 2023

Attolight-NTappy left a comment •

edited

Loading

Attolight-NTappy Apr 25, 2023

Fix bug in _build_spectrum #66

Fix bug in _build_spectrum #66

Conversation

LMSC-NTappy commented Nov 17, 2022 • edited Loading

Description of the change

Progress of the PR

Minimal example of the bug fix or the new feature

codecov bot commented Nov 17, 2022 • edited Loading

Codecov Report

jlaehne commented Nov 17, 2022

ericpre commented Feb 11, 2023

LMSC-NTappy commented Feb 13, 2023

jlaehne commented Apr 25, 2023

Attolight-NTappy left a comment • edited Loading

Choose a reason for hiding this comment

Attolight-NTappy Apr 25, 2023

Choose a reason for hiding this comment

LMSC-NTappy commented Nov 17, 2022 •

edited

Loading

codecov bot commented Nov 17, 2022 •

edited

Loading

Attolight-NTappy left a comment •

edited

Loading