Skip to content

fix(seviri): include the first IR row when converting RSR data - #294

Open
Manny7717 wants to merge 2 commits into
pytroll:mainfrom
Manny7717:fix/seviri-ir-first-row
Open

Manny7717 wants to merge 2 commits into
pytroll:mainfrom
Manny7717:fix/seviri-ir-first-row

Conversation

@Manny7717

Copy link
Copy Markdown

What

Fixes #253: the SEVIRI RSR converter skips the first IR spreadsheet data row. The issue report was correct: xlrd is 0-indexed, so spreadsheet line 13 corresponds to start_rowx=12, not 13.

For IR7.3 in the bundled workbook, row 12 starts at wavelength 6.35, while the current loader starts at row 13 and incorrectly begins at 6.37.

Change

In rsr_convert_scripts/seviri_rsr.py, the IR-channel branch now reads rows 12..111 instead of 13..112 for the wavelength column and every Meteosat response column.

Verification

  • Workbook proof from pyspectral/data/MSG_SEVIRI_Spectral_Response_Characterisation.XLS:
    • row 12 col 0 = 6.35
    • row 13 col 0 = 6.37
  • New regression test test_ir_channels_include_the_first_spreadsheet_row
    • fails before the fix: loaded first wavelength is 6.37
    • passes after the fix
  • Test runs:
    • .venv/bin/python -m pytest pyspectral/tests/test_seviri_rsr.py pyspectral/tests/test_config.py -q -> 4 passed
    • .venv/bin/flake8 rsr_convert_scripts/seviri_rsr.py pyspectral/tests/test_seviri_rsr.py pyspectral/tests/test_config.py -> passed

Why this is scoped correctly

This change only touches the IR-specific row window in the shared loader path. The non-IR channels already use start_rowx=12, and HRV uses a different block entirely.

xlrd is 0-indexed, so spreadsheet line 13 corresponds to row index
12. The IR-channel loader started at row 13 instead, skipping the
first wavelength/response entry (for example IR7.3 starts at 6.35 in
the workbook but loaded as 6.37). Read rows 12..111 for the IR block
and add a regression test against the bundled workbook.

Closes pytroll#253
@Manny7717

Copy link
Copy Markdown
Author

pre-commit.ci was right — my new test file was missing module/function docstrings (D100/D103 under flake8-docstrings). Added them in 7eb0722 and verified locally with the exact hook environment (flake8 + docstrings + bugbear + debugger): clean, and the regression test still passes.

@djhoese djhoese left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@adybbroe will need to be the final say on this pull request, especially since it will mean regenerating the RSR data and uploading it to the LUT data host. I think this looks good from what I understand except that this would be the first RSR script with tests. These aren't really user facing and are run "once" so we don't typically have tests for them. I almost want to say the test module should be removed, but @adybbroe should make the final call on this too. Thanks for fixing this @Manny7717.

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.

Missing first line of SEVIRI RSR

2 participants