i.spectral: fix text output to write band values#7488
Open
Valyrian-Code wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes the i.spectral -t text output regression where tuple representations were written instead of band values, and adds a regression test to prevent recurrence.
Changes:
- Update
write2textf()to write only band values (and correct pick numbering) for text output. - Add a pytest regression test validating the expected text output format for a simple 3-band case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| scripts/i.spectral/i.spectral.py | Fixes text output writing logic to avoid (index, row) tuple output and emit band values instead. |
| scripts/i.spectral/tests/i_spectral_test.py | Adds regression test covering the corrected -t text output formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
write2textf iterated with 'for row in enumerate(what)', binding row to the (index, row) tuple, so the -t text output wrote a Python tuple repr (a redundant index plus the easting/northing/label columns) instead of the band values. Unpack the index and write row[3:], matching draw_gnuplot and draw_linegraph, via writelines. Add a regression test for the -t output.
bf27a7b to
bf2a48a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
write2textfiterated withfor row in enumerate(what), which bindsrowtothe
(index, row)tuple. As a result the-ttext output wrote a Python tuplerepr — including a redundant index and the easting/northing/label columns —
instead of the band values:
This unpacks the index and writes
row[3:](the band DN values), matching whatdraw_gnuplotanddraw_linegraphalready do, so the output becomes:Motivation and context
The text export silently produced unusable data (a Python repr rather than the
spectral values shown in the plots). The exit code was still 0, so nothing
flagged it.
How has this been tested?
Reproduced locally before and after the change. Added a regression test
(
scripts/i.spectral/tests/i_spectral_test.py) that runsi.spectral -tonthree constant-value rasters and asserts the output line is
1, [10.0, 20.0, 30.0]; it fails on the previous code and passes on the fix.ruff formatandruff checkare clean.Types of changes